ardupilot
ardupilot copied to clipboard
AP_Camera: Load CAMERA_INFORMATION and VIDEO_STREAM_INFORMATION from Lua
This PR allows us to populate the CAMERA_INFORMATION and VIDEO_STREAM_INFORMATION MAVLink messages from Lua scripts.
Sending these messages allows a GCS to auto-configure the video stream to receive video from the payload.
Replaces https://github.com/ArduPilot/ardupilot/pull/27594.
Binary Name Text [B] Data [B] BSS (B) Total Flash Change [B] (%) Flash Free After PR (B)
--------------- --------------- ----------- ------------- ---------------------------- -------------------------
antennatracker 3516 (+0.2642%) 0 (0.0000%) 4 (+0.0015%) 3516 (+0.2638%) 629700
arduplane 3436 (+0.1915%) 0 (0.0000%) 4 (+0.0015%) 3436 (+0.1911%) 164640
blimp 3520 (+0.2602%) 0 (0.0000%) 0 (0.0000%) 3520 (+0.2598%) 607480
arducopter-heli 3428 (+0.1908%) 0 (0.0000%) -4 (-0.0015%) 3428 (+0.1904%) 162216
arducopter 3428 (+0.1907%) 0 (0.0000%) 4 (+0.0015%) 3428 (+0.1903%) 161344
ardusub 3432 (+0.2158%) 0 (0.0000%) 0 (0.0000%) 3432 (+0.2155%) 370092
ardurover 3428 (+0.2080%) 0 (0.0000%) -4 (-0.0015%) 3428 (+0.2077%) 312296
If scripting has to set it in anycase why not use scripting for the whole thing? You can receive and send MAVLink messages from Lua.
@IamPete1:
If scripting has to set it in anycase why not use scripting for the whole thing? You can receive and send MAVLink messages from Lua.
Two reasons:
- At least for the
CAMERA_INFORMATIONmessage, theAP_Cameradriver is already sending this message. I didn't want a situation where theAP_Cameradriver is sending the message and the scripting is sending a conflicting message. - These messages are requested rather than streamed. Is there a nice way to handle this in a script, rather than just periodically sending the message?
@Hwurzburg:
definitely needs to be disabled in minimize include and build_options/extract_featues files added
Done.
@Hwurzburg:
needs squash and library split
What do you mean?
- the commits are already split per library.
- the commits are logical changes that add one thing at a time, rather than a number of unrelated changes in a single commit.
- These messages are requested rather than streamed. Is there a nice way to handle this in a script, rather than just periodically sending the message?
There is a way to do this, I actually did exactly that and was about to open a PR for it, but I also was thinking about redoing it to something like your approach here. It's easy enough to intercept a MAV_CMD_REQUEST_VIDEO_STREAM_INFORMATION, but that's being deprecated in favor of MAV_CMD_REQUEST_MESSAGE. The issue with the latter is that only one message request is cached for the lua script to grab, so I had to run the script at high speed to make sure I didn't miss a message request if the GCS sent a bunch of them all at once.
So, I think there are some advantages to your approach.
My lua script for reference video_stream.zip
only one commit per library is allowed in final pr
squash to one commit then run Tools/gittools/git-subsystems-split script and push back
only one commit per library is allowed in final pr
This is definitely not true and IMO reduces clarity.
@Hwurzburg, @peterbarker: Squashed.
@peterbarker: Is FenceFloorEnabledLanding a flapping test? No code change, but it's now failing?
Its a new test - I haven't seen it flap
I can't trigger tests, so I've rebased on master again to trigger a rebuild.
Looks like this is now overflowing the heap on the CubeOrange-EKF2 build.
Also the AP_CAMERA_INFO_FROM_SCRIPT_ENABLED AP_MAVLINK_MSG_VIDEO_STREAM_INFORMATION_ENABLED might need to be in the scripting bindings too? You can do a depends on methods too if you want the main scripting support.
@IamPete1: I've updated the Lua bindings for the new structs and methods to depend on AP_CAMERA_INFO_FROM_SCRIPT_ENABLED.
Rebased on master to get the changes from #28128 so CI passes.
Binary Name Text [B] Data [B] BSS (B) Total Flash Change [B] (%) Flash Free After PR (B)
--------------- --------------- ----------- ------------- ---------------------------- -------------------------
antennatracker 3512 (+0.2622%) 0 (0.0000%) 0 (0.0000%) 3512 (+0.2618%) 620996
arducopter 3428 (+0.1896%) 0 (0.0000%) 4 (+0.0015%) 3428 (+0.1892%) 150640
ardusub 3428 (+0.2142%) 0 (0.0000%) 4 (+0.0015%) 3428 (+0.2139%) 359756
blimp 3512 (+0.2579%) 0 (0.0000%) 0 (0.0000%) 3512 (+0.2575%) 598928
arduplane 3428 (+0.1899%) 0 (0.0000%) 4 (+0.0015%) 3428 (+0.1895%) 153320
ardurover 3432 (+0.2070%) 0 (0.0000%) 0 (0.0000%) 3432 (+0.2067%) 302104
arducopter-heli 3428 (+0.1897%) 0 (0.0000%) -4 (-0.0015%) 3428 (+0.1893%) 151600
It's still big, but if the ifdefs work now, it should not be a problem.
Running locally, I get the following:
This branch, AP_CAMERA_INFO_FROM_SCRIPT_ENABLED = 0:
Build directory: /ardupilot/build/sitl
Target Text (B) Data (B) BSS (B) Total Flash Used (B) Free Flash (B) External Flash Used (B)
----------------------------------------------------------------------------------------------------------
bin/arducopter 4172858 191517 222752 4364375 Not Applicable Not Applicable
master branch:
Build directory: /ardupilot/build/sitl
Target Text (B) Data (B) BSS (B) Total Flash Used (B) Free Flash (B) External Flash Used (B)
----------------------------------------------------------------------------------------------------------
bin/arducopter 4172978 191453 222752 4364431 Not Applicable Not Applicable
Deltas between disabled and master:
Target Text (B) Data (B) BSS (B) Total Flash Used (B) Free Flash (B) External Flash Used (B)
----------------------------------------------------------------------------------------------------------
bin/arducopter -120 64 0 -56 Not Applicable Not Applicable
only one commit per library is allowed in final pr
This is definitely not true and IMO reduces clarity.
then we definitely need a change to developer WIKI instructions... @peterbarker ?: "The commits of the change should be squashed (see Interactive Rebase: cleaning up commits) into one commit and then the “Tools/gittools/git-subsystems-split” script run to create a single commit for each library module affected.
@Hwurzburg, I think the requirement is that individual commits should not affect more than one subsystem but multiple commits affecting a single subsystem is OK. The git-subsystems-split script is probably meant to divide up a single commit but not to consolidate commits.