Skip to content
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

Update capture.cpp to log camera serial number and configurable ID field #499

Merged
merged 2 commits into from
Sep 28, 2021

Conversation

EricClaeys
Copy link
Collaborator

Fixes #484. Fixes #498
Adds camera ID (for USB 3 cameras) and camera serial number to log output. These will be used in a future update that has different settings for different cameras.

Fixes #484.  Fixes #498
Adds camera ID (for USB 3 cameras) and camera serial number to log output.  These will be used in a future update that has different settings for different cameras.
Copy link
Collaborator

@ckuethe ckuethe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. I like where this is going.

capture.cpp Outdated Show resolved Hide resolved
capture.cpp Show resolved Hide resolved
capture.cpp Outdated Show resolved Hide resolved
capture.cpp Outdated Show resolved Hide resolved
capture.cpp Outdated Show resolved Hide resolved
capture.cpp Outdated Show resolved Hide resolved
@EricClaeys
Copy link
Collaborator Author

Chris, how do I submit a new commit to PR #499 to fix the issues you identified?

@ckuethe
Copy link
Collaborator

ckuethe commented Sep 24, 2021

Not sure how to do that with the web ui, but it's easy at the CLI.

It looks like you have a working branch for this change, so I assume you have git and the allsky source on your Pi. But you don't need to work on your Pi - you could do this inside a VM on your laptop/desktop. Anyway, you just keep committing and pushing changes on that branch until it's ready to be merged.

Github's docs are pretty good:

Maybe there's something in the github docs about making changes using the github Web UI.

@ckuethe
Copy link
Collaborator

ckuethe commented Sep 25, 2021

@EricClaeys would you be offended if I take over this PR and either try fix it, or close/reject it and make a new PR?

@EricClaeys
Copy link
Collaborator Author

@ckuethe no problem if you want to fix it. I view us as a team all working toward the same goal, so it's not too important to me who does what. Thanks for asking.

@linuxkidd please take a look at my newest capture_RPiHQ.cpp prior to refactoring. It's has many of the same functions as capture.cpp.

Eric

@EricClaeys
Copy link
Collaborator Author

@ckuethe I just got to the hotel room and went ahead and made the fixes

@EricClaeys EricClaeys requested a review from ckuethe September 28, 2021 01:41
Copy link
Collaborator

@ckuethe ckuethe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The -h/-help/--help behavior looks good now, and my camera is detected and identified. But I get a crash if I run capture with no arguments.

./capture

 ******************************************
 *** Allsky Camera Software v0.8.1 | 2021 ***
 ******************************************

Capture images of the sky with a Raspberry Pi and an ASI Camera

Add -h or --help for available options

Author: Thomas Jacquin - <[email protected]>

Contributors:
-Knut Olav Klo
-Daniel Johnsen
-Yang and Sam from ZWO
-Robert Wagner
-Michael J. Kidd - <[email protected]>
-Chris Kuethe

Segmentation fault (core dumped)

@EricClaeys
Copy link
Collaborator Author

It took me a long time to add the core dump feature, and now you're complaining about it? :-)
Since capture is currently never called with no arguments, you mind if I fix that bug in a future PR?
I think capture should use all the defaults specified in the code if no arguments are given.

@ckuethe
Copy link
Collaborator

ckuethe commented Sep 28, 2021

I'll approve to unblock. I agree that it should do something nice, such as run with sensible defaults or emit the help message when no args are given, but crashing is maybe not the most useful thing.

@ckuethe ckuethe merged commit a9f788e into master Sep 28, 2021
@EricClaeys EricClaeys deleted the Camera_ID_serialNumber_plus2fixes branch September 28, 2021 23:50
@ckuethe ckuethe changed the title Update capture.cpp Update capture.cpp to log camera serial number and configurable ID field Sep 29, 2021
@ckuethe
Copy link
Collaborator

ckuethe commented Sep 29, 2021

Interesting, The 120MM-S on my roof doesn't seem to support the serial number field.

Sep 29 13:35:56 skycam allsky.sh[3179]: *** WARNING: unable to get serialNumber (16 (ASI_ERROR_GENERAL_ERROR))

That doesn't stop capture from running properly, but I guess we will need to be a little bit careful about assuming that a serial number will be present.

@EricClaeys
Copy link
Collaborator Author

EricClaeys commented Sep 29, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

// define USE_HISTOGRAM stills needs line 92. help mode is broken
2 participants