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

Camera cleanups #8685

Merged

Conversation

WickedShell
Copy link
Contributor

@WickedShell WickedShell commented Jun 19, 2018

This is a set of random collection stuff I stumbled across while chasing something completely unrelated. Taken in no particular order:

  1. Adds the reboot required tag to CAM_FEEDBACK_PIN, (reboots are always required to detect shifts if it involved the edge feedback pin).
  2. Extracts the configuration of pin modes/pull ups from feedback_pin_timer and does it before the timer is installed. This is run at 1 kHz, so it's worth keeping this reasonably short, and with the reboot required if you shift pins under some circumstances I didn't feel that this was a large behavior shift.
  3. Set _camera_triggered to false on boot. I believe this fixes the double feedback event when taking the first photo.
  4. Removed some unneeded initialization to reduce binary size
  5. Renamed AP_Camera::check_trigger_pin(void) to AP_Camera::check_feedback_pin(void) to better reflect what it's trying to do. (This is still a very confusing section of code, this is just where I drew the line for limiting refactoring).

I need to test this (probably tomorrow) before we look at merging, but I'm most hopeful about 3, as the double feedback event is really frustrating.

// install a 1kHz timer to check feedback pin
hal.scheduler->register_timer_process(FUNCTOR_BIND_MEMBER(&AP_Camera::feedback_pin_timer, void));
int8_t dpin = hal.gpio->analogPinToDigitalPin(_feedback_pin);
if (dpin == -1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

A bit too much whitespaces

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ha! Not really sure how I did that, will fix.

@WickedShell WickedShell force-pushed the wickedshell/cam-feedback-tidy branch from 2d22d72 to c0dc4c7 Compare June 23, 2018 03:19
@WickedShell WickedShell removed the WIP label Jun 29, 2018
@WickedShell
Copy link
Contributor Author

Works correctly, does not fix the double CAM message on first trigger.

@WickedShell WickedShell force-pushed the wickedshell/cam-feedback-tidy branch from c0dc4c7 to d6e08e0 Compare July 9, 2018 23:13
@WickedShell WickedShell merged commit f8e9e57 into ArduPilot:master Jul 9, 2018
@WickedShell
Copy link
Contributor Author

Passed dev call review, was waiting on CI to pass so merging.

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

Successfully merging this pull request may close these issues.

2 participants