-
Notifications
You must be signed in to change notification settings - Fork 49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Camera updates #589
Camera updates #589
Conversation
* cr2 to jpg: * Cleanup and add docs * Caption is optional (it's a slow operation) * No resizing of jpg
scripts/cr2_to_jpg.sh
Outdated
|
||
FNAME=$1 | ||
NAME=$2 | ||
CAPTION=$2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please consider running shellcheck on this and following much of its advice. Here it would probably recommend quoting $2. I would recommend: CAPTION="${2}"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't even know of shellcheck! I don't do much actual bash scripting. Thanks!
pocs/camera/canon_gphoto2.py
Outdated
@@ -35,6 +35,8 @@ def connect(self): | |||
|
|||
# Properties to be set upon init. | |||
prop2index = { | |||
'/main/settings/datetime': 'now', # Current datetime |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about sorting these lines lexically?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Not sure it makes it more clear but a good idea regardless.
scripts/cr2_to_jpg.sh
Outdated
|
||
FNAME=$1 | ||
NAME=$2 | ||
CAPTION=$2 | ||
|
||
JPG=${FNAME/cr2/jpg} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about: JPG="${FNAME%.cr2}.jpg"
This will only replace .cr2
at the end of the string, not in the middle.
scripts/cr2_to_jpg.sh
Outdated
|
||
JPG=${FNAME/cr2/jpg} | ||
|
||
echo "Converting ${FNAME} to jpg." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jpg was probably supposed to be ${JPG}
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought it might be too long to and redundant to have the full path for both since we don't offer the possibility of a different name or output folder but since that's not explicit it's probably best to clarify.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I changed it to "Converting CR2 to ${JPG}".
scripts/cr2_to_jpg.sh
Outdated
# Make thumbnail from jpg. | ||
convert ${JPG} -thumbnail 1280x1024 -background black -fill red \ | ||
-font ubuntu -pointsize 24 label:"${NAME}" -gravity South -append ${JPG} | ||
if [[ $CAPTION ]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about: if [[ -n "${CAPTION}" ]]
scripts/take_pic.sh
Outdated
--wait-event=${T}s --set-config eosremoterelease=4 --wait-event-and-download=2s \ | ||
--filename "${F}" | ||
gphoto2 --port=${PORT} \ | ||
--set-config shutterspeed=0 \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add comments at the end of the lines where we have these magic numbers and symbols?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have a better syntax than this?
# Open shutter
gphoto2 --port=${PORT} \
--set-config shutterspeed=0 `#Always set to bulb` \
--set-config capturetarget=0 `#Capture to RAM for download` \
--set-config eosremoterelease=Immediate \
scripts/take_pic.sh
Outdated
Options: | ||
PORT USB port as reported by gphoto2 --auto-detect, e.g. usb:001,004. | ||
EXPTIME Exposure time in seconds, should be greater than 1 second. | ||
OUTFILE Output filename (with .cr2 extension). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With "appropriate" extension; e.g. .cr2 for Canon RAW images.
* changing the `artist` to be the unit_id and cleanup to some properties.
* Adding multiline comments for magic numbers
We have made some changes recently that should be marked with different version number. Specifically, panoptes#589 will standardize the DATE-OBS headers in the images. At some point we will need to scrub the data created prior to that so we want to be able to delineate.
We have made some changes recently that should be marked with different version number. Specifically, #589 will standardize the DATE-OBS headers in the images. At some point we will need to scrub the data created prior to that so we want to be able to delineate.
Ideally the take_pic.sh script will be re-written in python to handle any pictures (e.g. bias and regular) but this is some small cleanup before that time.
Note that this was also waiting to get some of the camera hardware testing infrastructure in place but I wanted to make sure to get these updates out.