Added support for gphoto2 external camera types Nikon or Canon Camaras#201
Open
MoltenOre wants to merge 2 commits into
Open
Added support for gphoto2 external camera types Nikon or Canon Camaras#201MoltenOre wants to merge 2 commits into
MoltenOre wants to merge 2 commits into
Conversation
…by allowing users to specify the camera model in the configuration. The code now builds and executes a gphoto2 command to capture and download images directly from the camera, improving compatibility and performance for supported devices.
MoltenOre
changed the title
MoltenOre
changed the title
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughThe PR adds gphoto2 camera support to Moonraker's Timelapse component as an optional alternative to wget-based capture. It introduces a Changesgphoto2 Camera Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/configuration.md (1)
243-303:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse one consistent
moonraker.confassignment style across examples.Line 243 uses
gphoto2_camera_model:while Line 302 usesgphoto2_camera_model = .... Pick one style consistently to avoid user misconfiguration during copy/paste.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/configuration.md` around lines 243 - 303, The docs use two styles for the moonraker setting (gphoto2_camera_model: vs gphoto2_camera_model =); pick one style and make all examples consistent—search for the symbol gphoto2_camera_model in this document and replace occurrences so every example uses the same assignment form (either use "gphoto2_camera_model = <value>" everywhere or "gphoto2_camera_model: <value>" everywhere), and ensure the final code block and explanatory text match that chosen format.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@component/timelapse.py`:
- Around line 475-505: The gphoto2 and wget command strings are built from
writable settings (self.config['gphoto2_camera_model'],
self.config['snapshoturl']) and passed to shell_command.build_shell_command,
creating a shell injection/RCE risk; update the capture logic in
component/timelapse.py to pass arguments safely instead of interpolating raw
strings: for gphoto2 use an argv-style invocation or properly quote the camera
model (e.g., use a list of args or shlex.quote) when constructing the command,
and for the snapshot use a safe download API or pass the URL as a single
argument (or quoted) rather than concatenating into a shell string, and ensure
tempfile path (self.temp_dir + framefile) is joined and sanitized; finally,
modify calls around build_shell_command and scmd.run to accept the argv form (or
properly escaped string) so untrusted values cannot inject additional shell
operators.
In `@docs/configuration.md`:
- Around line 294-296: Update the spelling of the gphoto2 setup steps by
correcting the misspelled tokens: change "Camara" to "Camera", "timelaps" to
"timelapse", and "menue" to "menu" in the sentence that reads "Select that
Camara in the timelaps menue" and ensure the surrounding guidance still reads
clearly about adding a dummy webcam and setting Stream Delay Compensation.
In `@docs/installation.md`:
- Around line 56-69: Fix the user-facing typos and the malformed example
command: change the heading text "Install gphoto2 if an External camara is to be
used like a Nikon D40" to use correct spelling/casing (e.g., "Install gphoto2 if
an external camera is used (e.g., Nikon D40)") and correct "camara"→"camera";
update the example gphoto2 command `gphoto2 --camera '{Model}}'
--capture-image-and-download --filename '/tmp/test_Image000001.jpg'` to remove
the extra brace so the camera placeholder is `{Model}` (e.g., `--camera
'{Model}'`) and ensure the command is shown as a copy/paste-ready shell example
in the docs.
---
Outside diff comments:
In `@docs/configuration.md`:
- Around line 243-303: The docs use two styles for the moonraker setting
(gphoto2_camera_model: vs gphoto2_camera_model =); pick one style and make all
examples consistent—search for the symbol gphoto2_camera_model in this document
and replace occurrences so every example uses the same assignment form (either
use "gphoto2_camera_model = <value>" everywhere or "gphoto2_camera_model:
<value>" everywhere), and ensure the final code block and explanatory text match
that chosen format.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 381fb1bc-3abb-40b8-9c7f-5137277ca06c
📒 Files selected for processing (3)
component/timelapse.pydocs/configuration.mddocs/installation.md
Comment on lines
+475
to
+505
| # Route to appropriate camera handler | ||
| if self.config['gphoto2_camera_model'] != "": | ||
| framepath = os.path.join(self.temp_dir, framefile) | ||
| # Build gphoto2 command | ||
| # --camera selects which camera to use needs model from gphoto2 --auto-detect | ||
| # --capture-image-and-download folder/file captures and downloads in one operation | ||
| cmd = f"gphoto2 --camera '{self.config['gphoto2_camera_model']}' --capture-image-and-download --filename '{framepath}'" | ||
| logging.debug(f"Executing gphoto2 command: {cmd}") | ||
|
|
||
| shell_cmd: SCMDComp = self.server.lookup_component('shell_command') | ||
| scmd = shell_cmd.build_shell_command(cmd, None) | ||
|
|
||
| try: | ||
| cmdstatus = await scmd.run(timeout=5., verbose=False) | ||
| except Exception: | ||
| logging.exception(f"Error executing gphoto2 command: {cmd}") | ||
|
|
||
| else: | ||
| # Default webcam capture via wget | ||
| options = "" | ||
| if self.wget_skip_cert: | ||
| options += "--no-check-certificate " | ||
|
|
||
| cmd = "wget " + options + self.config['snapshoturl'] \ | ||
| + " -O " + self.temp_dir + framefile | ||
| logging.debug(f"cmd: {cmd}") | ||
|
|
||
| shell_cmd: SCMDComp = self.server.lookup_component('shell_command') | ||
| scmd = shell_cmd.build_shell_command(cmd, None) | ||
| try: | ||
| cmdstatus = await scmd.run(timeout=2., verbose=False) |
There was a problem hiding this comment.
Prevent shell command injection in capture command construction.
Line 481 and Lines 498-499 build shell commands from configurable values and execute them. Because gphoto2_camera_model and snapshoturl are writable settings, this creates an RCE path via shell injection.
Suggested hardening
@@
import re
+import shlex
@@
- cmd = f"gphoto2 --camera '{self.config['gphoto2_camera_model']}' --capture-image-and-download --filename '{framepath}'"
+ model = shlex.quote(self.config['gphoto2_camera_model'])
+ output_file = shlex.quote(framepath)
+ cmd = (
+ "gphoto2 "
+ f"--camera {model} "
+ "--capture-image-and-download "
+ f"--filename {output_file}"
+ )
@@
- cmd = "wget " + options + self.config['snapshoturl'] \
- + " -O " + self.temp_dir + framefile
+ snapshot_url = shlex.quote(self.config['snapshoturl'])
+ output_file = shlex.quote(os.path.join(self.temp_dir, framefile))
+ cmd = f"wget {options}{snapshot_url} -O {output_file}"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@component/timelapse.py` around lines 475 - 505, The gphoto2 and wget command
strings are built from writable settings (self.config['gphoto2_camera_model'],
self.config['snapshoturl']) and passed to shell_command.build_shell_command,
creating a shell injection/RCE risk; update the capture logic in
component/timelapse.py to pass arguments safely instead of interpolating raw
strings: for gphoto2 use an argv-style invocation or properly quote the camera
model (e.g., use a list of args or shlex.quote) when constructing the command,
and for the snapshot use a safe download API or pass the URL as a single
argument (or quoted) rather than concatenating into a shell string, and ensure
tempfile path (self.temp_dir + framefile) is joined and sanitized; finally,
modify calls around build_shell_command and scmd.run to accept the argv form (or
properly escaped string) so untrusted values cannot inject additional shell
operators.
Comment on lines
+294
to
+296
| Add a dummy webcam and give it a stream and snapshot URL that is not in use. | ||
| Select that Camara in the timelaps menue. | ||
| Set the Stream Delay Compensation to the amount of time it takes to focus and take a picture (setting it to manual focus decreases the time taken) |
There was a problem hiding this comment.
Correct spelling in the gphoto2 setup steps.
Line 295 has multiple typos (“Camara”, “timelaps”, “menue”), which makes the instructions look unreliable.
🧰 Tools
🪛 LanguageTool
[grammar] ~295-~295: Ensure spelling is correct
Context: ...hot URL that is not in use. Select that Camara in the timelaps menue. Set the Stream D...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~295-~295: Ensure spelling is correct
Context: ...s not in use. Select that Camara in the timelaps menue. Set the Stream Delay Compensation to t...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/configuration.md` around lines 294 - 296, Update the spelling of the
gphoto2 setup steps by correcting the misspelled tokens: change "Camara" to
"Camera", "timelaps" to "timelapse", and "menue" to "menu" in the sentence that
reads "Select that Camara in the timelaps menue" and ensure the surrounding
guidance still reads clearly about adding a dummy webcam and setting Stream
Delay Compensation.
Source: Linters/SAST tools
Comment on lines
+56
to
+69
| ## Install gphoto2 if an External camara is to be used like a Nikon D40 | ||
|
|
||
| ```bash | ||
| # Debian/Ubuntu/Raspbian | ||
| sudo apt-get update | ||
| sudo apt-get install gphoto2 | ||
| ``` | ||
|
|
||
| # Support for gphoto2 | ||
|
|
||
| For issues: | ||
| 1. Check `moonraker.log` for errors | ||
| 2. Test gphoto2 manually: `gphoto2 --camera '{Model}}' --capture-image-and-download --filename '/tmp/test_Image000001.jpg'` | ||
| 3. Verify camera compatibility at: http://gphoto.org/proj/libgphoto2/support.php No newline at end of file |
There was a problem hiding this comment.
Fix setup text typos and malformed troubleshooting command.
Line 56 (“camara”) and Line 68 ({Model}}) contain user-facing errors; the command example is currently copy/paste incorrect.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/installation.md` around lines 56 - 69, Fix the user-facing typos and the
malformed example command: change the heading text "Install gphoto2 if an
External camara is to be used like a Nikon D40" to use correct spelling/casing
(e.g., "Install gphoto2 if an external camera is used (e.g., Nikon D40)") and
correct "camara"→"camera"; update the example gphoto2 command `gphoto2 --camera
'{Model}}' --capture-image-and-download --filename '/tmp/test_Image000001.jpg'`
to remove the extra brace so the camera placeholder is `{Model}` (e.g.,
`--camera '{Model}'`) and ensure the command is shown as a copy/paste-ready
shell example in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added support for gphoto2 external camera types like Nikon or Canon by allowing users to specify the camera model in the configuration. The code now builds and executes a gphoto2 command to capture and download images directly from the camera, improving compatibility and performance for supported devices.