AP_Camera: Load CAMERA_INFORMATION and VIDEO_STREAM_INFORMATION from Lua

[nexton-winjeel]

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 #27594.

[amilcarlucas]

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

[Hwurzburg]

definitely needs to be disabled in minimize include and build_options/extract_featues files added

[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.

[nexton-winjeel]

@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:

1. At least for the CAMERA_INFORMATION message, the AP_Camera driver is already sending this message. I didn't want a situation where the AP_Camera driver is sending the message and the scripting is sending a conflicting message.
2. 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?

[nexton-winjeel]

@Hwurzburg:

definitely needs to be disabled in minimize include and build_options/extract_featues files added

Done.

[Hwurzburg]

needs squash and library split

[nexton-winjeel]

@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.

[robertlong13]

2. 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

[Hwurzburg]

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

[tpwrules]

only one commit per library is allowed in final pr

This is definitely not true and IMO reduces clarity.

[peterbarker]

only one commit per library is allowed in final pr

That's not the case. But there are too many commits in here.

This is a problem, 'though, @nexton-winjeel

I usually also squash the changes to build_options.py and extract_features.py into a single commit (and often just omit the fact it touches extract_features.py) - there are two examples of that in the current commit list.

I'd also squash these two:

These could possibly also be squashed:

... they can be separated, but it's not particularly useful (and this, I think, the sort of thing that Henry might be kind of objecting to).

[nexton-winjeel]

@Hwurzburg, @peterbarker: Squashed.

[nexton-winjeel]

@peterbarker: Is FenceFloorEnabledLanding a flapping test? No code change, but it's now failing?

[andyp1per]

Its a new test - I haven't seen it flap

[nexton-winjeel]

I can't trigger tests, so I've rebased on master again to trigger a rebuild.

[tridge]

we should bring in the XML for camera_id

[nexton-winjeel]

@tridge: Is this PR blocked on getting the MAVLink XML changes for camera_id brought in?

[nexton-winjeel]

Looks like this is now overflowing the heap on the CubeOrange-EKF2 build.

[IamPete1]

Depends should match the AP_Camera one.

[IamPete1]

Suggested change

	userdata mavlink_video_stream_information_t depends AP_CAMERA_ENABLED
	userdata mavlink_video_stream_information_t depends AP_CAMERA_ENABLED && (AP_CAMERA_SCRIPTING_ENABLED == 1)

[IamPete1]

Suggested change

	userdata mavlink_camera_information_t depends AP_CAMERA_ENABLED
	userdata mavlink_camera_information_t depends AP_CAMERA_ENABLED && (AP_CAMERA_SCRIPTING_ENABLED == 1)

[IamPete1]

Suggested change

	return set_video_stream_information()
	return set_video_stream_information()

[IamPete1]

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.

[nexton-winjeel]

@IamPete1: I've updated the Lua bindings for the new structs and methods to depend on AP_CAMERA_INFO_FROM_SCRIPT_ENABLED.

[nexton-winjeel]

Rebased on master to get the changes from #28128 so CI passes.

[amilcarlucas]

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.

[nexton-winjeel]

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  

[Hwurzburg]

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.

[rmackay9]

@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.