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

AP_Camera: add CAMx_OPTION support for start/stop recording when arm/Disarm #24193

Merged
merged 1 commit into from Jul 4, 2023

Conversation

khanasif786
Copy link
Contributor

@khanasif786 khanasif786 commented Jul 2, 2023

Tested on SiYI zr10.

@khanasif786
Copy link
Contributor Author

khanasif786 commented Jul 2, 2023

I think you should let this one past, blocking this means that if the camera recording ever gets of of sync with what AP is expecting we can't fix it.

@IamPete1 Thanks for the suggestions. Well the problem with not blocking is that ( atleast what i see in my case in SiYi zr10) if we don't block then It sends recording message to camera again and again that causes camera record 0bytes mp4 files. May be it is due to the fact that SiYi has same message(command) to start and stop video recording. So every time we arm it causes multiple 0Bytes files to write in the memory in those not a single one is playable.

@khanasif786
Copy link
Contributor Author

@rmackay9 I think instead of values we should use a bitmask so in future adding more options will be easier.

@rmackay9
Copy link
Contributor

rmackay9 commented Jul 3, 2023

@khanasif786,

Great, this is much better thanks.

Can you fix the commit message to start with "AP_Camera:"?

@khanasif786
Copy link
Contributor Author

@rmackay9 I think instead of values we should use a bitmask so in future adding more options will be easier.

The required changes for this also done.

@rmackay9
Copy link
Contributor

rmackay9 commented Jul 3, 2023

@khanasif786,

Great, thanks very much for this.

This has been tested to confirm that the ZR10 attempts to start and stop recording when the autopilot is armed/disarmed? I'm sure you have but I just want to check before we merge.

@khanasif786
Copy link
Contributor Author

khanasif786 commented Jul 3, 2023

@rmackay9 Yeah Tested on Zr10.

@tridge tridge merged commit 82b7090 into ArduPilot:master Jul 4, 2023
79 of 81 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants