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.

Sign up for GitHub

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

Jump to bottom

AP_Mount, AP_Camera: Load CAMERA_INFORMATION from JSON #27594

Closed

nexton-winjeel wants to merge 19 commits into ArduPilot:master from Winjeel:upstream/create-camera_information-msg-from-json

Contributor

nexton-winjeel commented Jul 19, 2024

This PR allows us to add a *.json file to the SD card (or ROMFS) that is used to populate the CAMERA_INFORMATION and VIDEO_STREAM_INFORMATION MAVLink messages. These messages are sent for AP_Mount backends that do not already implement these messages.

Sending these messages allows a GCS to auto-configure the video stream to receive video from the payload.

Contributor

rmackay9 commented Jul 19, 2024

really great to see this!

Contributor Author

nexton-winjeel commented Jul 22, 2024

Two issues:

the test vectors in the new tests fail the formatter; and
the shutil.copy() calls in the new tests fail if the intermediate directories aren't present.

I'll fix them now...

nexton-winjeel added 8 commits

July 22, 2024 14:42


          AP_Mount: Add hooks to send VIDEO_STREAM_INFORMATION

4f42042


          AP_Camera: Add hooks to send VIDEO_STREAM_INFORMATION

ed2fffb


          GCS_MAVLink: Add handler for VIDEO_STREAM_INFORMATION request

8ce96cb


          AP_Camera: Add JSON files to define MAVLink message contents

18a58c8


          AP_HAL_ChibiOS: Add MAVLink JSON defs to ROMFS if present in hwdef fo…

f666a1f

…lder


          AP_Camera: Add #define to enable MAVLink msg content from JSON

b217d89


          AP_Mount: Update send_camera_information() to return bool

fb5b197

Allows base AP_Camera driver to send this message if not sent by the AP_Mount driver.


          AP_Mount: Update video_stream_information() to return bool

99c10e6

Allows base AP_Camera driver to send this message if not sent by the AP_Mount driver.

nexton-winjeel force-pushed the upstream/create-camera_information-msg-from-json branch from 01583e5 to fd46a2f Compare

July 22, 2024 04:44

nexton-winjeel marked this pull request as ready for review

July 22, 2024 04:46

Contributor Author

nexton-winjeel commented Jul 22, 2024

@rmackay9: I'll jump on the dev call tomorrow if you want to discuss this.

peterbarker requested changes

View reviewed changes

Contributor

peterbarker left a comment

We should make sure that when compiled out these new features take up approximately no space.

Won't be able to be no space given different return values for some methods now

libraries/AP_Camera/AP_Camera.cpp

                       CHECK_PAYLOAD_SIZE2(CAMERA_CAPTURE_STATUS);
                       send_camera_capture_status(chan);
                       break;
+                  case MSG_VIDEO_STREAM_INFO:

Contributor

peterbarker Jul 23, 2024

Suggested change

      
                case MSG_VIDEO_STREAM_INFO:
          
            AP_MAVLINK_MSG_VIDEO_STREAM_INFORMATION_ENABLED
          
                case MSG_VIDEO_STREAM_INFO:

.... and generally use it elsewhere. These are defined in GCS_config.h

Contributor Author

nexton-winjeel Jul 24, 2024

Done.

libraries/AP_Camera/AP_Camera_Backend.cpp Outdated

+              #endif
+              #if AP_CAMERA_JSON_INFO_ENABLED
+                  #include <AP_Filesystem/AP_Filesystem.h>

Contributor

peterbarker Jul 23, 2024

We don't usually indent these lines

Contributor Author

nexton-winjeel Jul 24, 2024

Fixed.

libraries/AP_Camera/AP_Camera_Backend.cpp Outdated

+              // fall back to looking for the same folder in ROMFS.
+              //
+              // This allocates a AP_JSON::value object that will need to be freed.
+              AP_JSON::value * AP_Camera_Backend::_load_mount_msg_json(const char* json_filename) {

Contributor

peterbarker Jul 23, 2024

Suggested change

      
            AP_JSON::value * AP_Camera_Backend::_load_mount_msg_json(const char* json_filename) {
          
            AP_JSON::value * AP_Camera_Backend::_load_mount_msg_json(const char* json_filename)
          
            {

similarly elsewhere

Contributor Author

nexton-winjeel Jul 24, 2024

Fixed.

libraries/AP_Camera/AP_Camera_Backend.cpp

+              // This allocates a AP_JSON::value object that will need to be freed.
+              AP_JSON::value * AP_Camera_Backend::_load_mount_msg_json(const char* json_filename) {
+                  char* romfs_json_path = nullptr;
+                  int alloc = asprintf(&romfs_json_path, "@ROMFS/mav_msg_def/AP_Camera/%s", json_filename);

Contributor

peterbarker Jul 23, 2024

Do without the local here?

Contributor Author

nexton-winjeel Jul 24, 2024

I find it more readable with the local.

libraries/AP_Camera/AP_Camera_Backend.cpp Outdated

+              }
+              // helper function to copy a JSON double into a msg struct
+              template <typename T> bool AP_Camera_Backend::_copy_json_field_double(const AP_JSON::value* obj, const char* key, T& dst) {

Contributor

peterbarker Jul 23, 2024

Suggested change

      
            template <typename T> bool AP_Camera_Backend::_copy_json_field_double(const AP_JSON::value* obj, const char* key, T& dst) {
          
            template <typename T>
          
            bool AP_Camera_Backend::_copy_json_field_double(const AP_JSON::value* obj, const char* key, T& dst) {

Contributor Author

nexton-winjeel Jul 24, 2024

Fixed.

libraries/AP_Camera/AP_Camera_Backend.cpp Outdated

+                  if (!_copy_json_field_string(obj, "cam_definition_uri", camera_info.msg.cam_definition_uri, sizeof(camera_info.msg.cam_definition_uri))) goto err;
+                  // Populate the fields that shouldn't change.
+                  camera_info.msg.time_boot_ms = AP_HAL::millis();

Contributor

peterbarker Jul 23, 2024

Errr.. this one should definitely change

Contributor Author

nexton-winjeel Jul 24, 2024

Fixed.

libraries/AP_Camera/AP_Camera_Backend.cpp Outdated

Comment on lines 620 to 622

		#undef GET_JSON_DOUBLE
		#undef GET_JSON_STR

Contributor

peterbarker Jul 23, 2024

Were these replaced by the template function?

Contributor Author

nexton-winjeel Jul 24, 2024

Yes, they were. Fixed.

libraries/AP_Camera/AP_Camera_Mount.cpp Outdated

               {
                   AP_Mount* mount = AP::mount();
                   if (mount != nullptr) {
-                      return mount->send_camera_information(get_mount_instance(), chan);
+                      bool sent_by_mount = mount->send_camera_information(get_mount_instance(), chan);

Contributor

peterbarker Jul 23, 2024

Suggested change

      
                    bool sent_by_mount = mount->send_camera_information(get_mount_instance(), chan);
          
                    const bool sent_by_mount = mount->send_camera_information(get_mount_instance(), chan);

Contributor Author

nexton-winjeel Jul 24, 2024

Fixed.

libraries/AP_Camera/AP_Camera_Mount.cpp Outdated

Comment on lines 124 to 125

		AP_Mount* mount = AP::mount();
		if (mount != nullptr) {

Contributor

peterbarker Jul 23, 2024

Suggested change

      
                AP_Mount* mount = AP::mount();
          
                if (mount != nullptr) {
          
                AP_Mount* mount = AP::mount();
          
                if (mount == nullptr) {
          
                    return;
          
                }

Contributor Author

nexton-winjeel Jul 24, 2024

Fixed.

libraries/AP_Camera/json/video_stream_information.json

		@@ -0,0 +1,20 @@
		# This file is used to populate the VIDEO_STREAM_INFORMATION message sent by

Contributor

peterbarker Jul 23, 2024

SHould we put these under an "examples" subdirectory as we do for a lot of other things?

Contributor Author

nexton-winjeel Jul 24, 2024

I'd prefer they live in the AP_Camera folder. The code in the backend makes assumptions about what fields are and aren't present in the JSON.

nexton-winjeel force-pushed the upstream/create-camera_information-msg-from-json branch from fd46a2f to 230a5ca Compare

July 24, 2024 03:15

nexton-winjeel requested a review from peterbarker

July 24, 2024 04:28

nexton-winjeel marked this pull request as draft

July 24, 2024 23:31

Contributor Author

nexton-winjeel commented Jul 24, 2024

Temporarily back to draft -- I need to confirm that this still works on hardware.

nexton-winjeel added 11 commits

July 25, 2024 10:46


          AP_Camera: In backend, load MAVLink msg content from JSON

b2fac7c

Loads the contents of the following messages from JSON files:
    - CAMERA_INFORMATION
    - VIDEO_STREAM_INFORMATION


          AP_Camera: In Mount backend, send messages from JSON if not sent by A…

84e92d8

…P_Mount


          AP_Camera: In CAMERA_INFORMATION JSON message, use lens_id to identif…

9dad119

…y instances


          AP_Camera: Remove support for multiple instances of JSON messages

6ba6f1f

ArduPilot doesn't disambiguate MAVLink Camera messages between backends, so don't bother supporting multiple JSON definitions.


          AP_Camera: In backend, use hal.console->printf() for debug printing

30a564f


          GCS_MAVLink: Add missing MSG_CAMERA_CAPTURE_STATUS to try_send_message()

bed49e9


          autotest: Add test for CAMERA_INFORMATION msg defined in JSON

8a44abe


          autotest: Add test for VIDEO_STREAM_INFORMATION msg defined in JSON

ceaeb71


          GCS_MAVLink: Add AP_MAVLINK_MSG_VIDEO_STREAM_INFORMATION_ENABLED #define

112184d


          AP_Camera: Use AP_MAVLINK_MSG_VIDEO_STREAM_INFORMATION_ENABLED guard

8403ca8


          AP_Mount: Use AP_MAVLINK_MSG_VIDEO_STREAM_INFORMATION_ENABLED guard

40a132f

nexton-winjeel force-pushed the upstream/create-camera_information-msg-from-json branch from 230a5ca to 40a132f Compare

July 25, 2024 00:47

Contributor Author

nexton-winjeel commented Jul 25, 2024

Running on a CubeOrange with debug enabled:

STABILIZE> reboot
STABILIZE> Got COMMAND_ACK: PREFLIGHT_REBOOT_SHUTDOWN: ACCEPTED
Device /dev/serial/by-id/usb-Hex_ProfiCNC_CubeOrange_24004E001351313139383237-if00 is dead
Device /dev/serial/by-id/usb-Hex_ProfiCNC_CubeOrange_24004E001351313139383237-if00 reopened OK
AP_Logger_File: buffer size=204800
MS5611 found on bus 4 address 0x02
MS5611 found on bus 1 address 0x03
AP: Initialising ArduPilot
disabling flow control on serial 1
_load_mount_msg_json:479: failed to load 'mav_msg_def/AP_Camera/camera_information.json'
init_camera_information_from_json:616: Loaded camera_info from 'camera_information.json'
init_camera_information_from_json:617:     camera_info.msg.time_boot_ms=0
init_camera_information_from_json:618:     camera_info.msg.vendor_name='Unknown'
init_camera_information_from_json:619:     camera_info.msg.model_name='Camera'
init_camera_information_from_json:620:     camera_info.msg.firmware_version=0
init_camera_information_from_json:621:     camera_info.msg.focal_length=1.600000
init_camera_information_from_json:622:     camera_info.msg.sensor_size_h=3840.000
init_camera_information_from_json:623:     camera_info.msg.sensor_size_v=2160.000
init_camera_information_from_json:624:     camera_info.msg.resolution_h=1920
init_camera_information_from_json:625:     camera_info.msg.resolution_v=1080
init_camera_information_from_json:626:     camera_info.msg.lens_id=1
init_camera_information_from_json:627:     camera_info.msg.flags=256
init_camera_information_from_json:628:     camera_info.msg.cam_definition_version=0
init_camera_information_from_json:629:     camera_info.msg.cam_definition_uri=''
init_camera_information_from_json:630:     camera_info.msg.gimbal_device_id=1
_load_mount_msg_json:479: failed to load 'mav_msg_def/AP_Camera/video_stream_information.json'
init_video_stream_information_from_json:560: Loaded video_stream_info from 'video_stream_information.json'
init_video_stream_information_from_json:561:     video_stream_info.msg.stream_id=1
init_video_stream_information_from_json:562:     video_stream_info.msg.count=1
init_video_stream_information_from_json:563:     video_stream_info.msg.type=1
init_video_stream_information_from_json:564:     video_stream_info.msg.flags=1
init_video_stream_information_from_json:565:     video_stream_info.msg.framerate=30.00000
init_video_stream_information_from_json:566:     video_stream_info.msg.resolution_h=1920
init_video_stream_information_from_json:567:     video_stream_info.msg.resolution_v=1080
init_video_stream_information_from_json:568:     video_stream_info.msg.bitrate=1500
init_video_stream_information_from_json:569:     video_stream_info.msg.rotation=0
init_video_stream_information_from_json:570:     video_stream_info.msg.hfov=50
init_video_stream_information_from_json:572:     video_stream_info.msg.name='Video'
init_video_stream_information_from_json:573:     video_stream_info.msg.uri='127.0.0.1:5600'
AP: Calibrating barometer
AP: Barometer 1 calibration complete
AP: Barometer 2 calibration complete
Init Gyro**
AP: ArduPilot Ready

nexton-winjeel marked this pull request as ready for review

July 25, 2024 00:49

nexton-winjeel mentioned this pull request

AP_Camera: Load CAMERA_INFORMATION and VIDEO_STREAM_INFORMATION from Lua #27794

Merged

Contributor Author

nexton-winjeel commented Aug 8, 2024

@rmackay9, @peterbarker: Closing in favour of #27794

nexton-winjeel closed this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet