Integrated Tusuav's gimbal driver and backend #28491

sunny-1987 · 2024-10-29T03:35:44Z

Tusuav Gimbal

Tusuav is a company specializing in the research and production of VTOLs,including the gimbals. The drive for its GT30 series gimbals has been added in these commits. The pods featured in this submission offer continuous zoom capability, EO camera and IR camera switching with picture-In-picture (PIP) functionality, target tracking, pan and tilt angle & rate control , photo-taking, video recording, and rangefinder up to 3km.

The protocol description

/*Field Index Bytes Description
-------------------------------------------------------------------------------------------
Header 0 1 0x5A
Length 1 1 total length, n=all bytes from byte5 to checksum, min=6, max=128
Flag 2 1 bit0-2: board id of the gimbal 0=main controller 1=comm board 2~4=motor pitch/roll/yaw
bit3: 1= need reply 0 = no need
bit4: 1= is a reply message 0 = now a reply message
bit5: 1= is a reading message 0 = is a writing message

Cmd 3 1 see details at below
Sub Cmd 4 1 see details at below
Data 5-n+1 n data in little endian lowest byte first
Checksum n+2 1 calculate_crc from mavlink 2.0 of byte from 0 to n+1 (inclusive)
*/

The website of Tusuav

https://www.tusuav.com/pod

rmackay9 · 2024-10-30T10:12:48Z

libraries/AP_Mount/AP_Mount_Tusuav.cpp

+const char* AP_Mount_Tusuav::send_text_prefix = "Tusuav:";
+
+// init - performs any required initialisation for this instance
+// void AP_Mount_Tusuav::init()


Thanks for the PR! We normally don't allow commented out code to be added so could you remove this?

Thanks for the feedback! We've removed the commented-out code as requested. Let us know if there’s anything else that needs adjustment

rmackay9 · 2024-10-30T10:13:30Z

libraries/AP_Mount/AP_Mount_Tusuav.cpp

+    uint32_t now_ms = AP_HAL::millis();
+
+
+    if(now_ms - _last_update_ms_parse >= AP_MOUNT_TUSUAV_PARSE_INTERVAL_MS)


small formatting issues here. There should be a space between the "if" and the bracket. Also the curly brace at the beginning of the next line should actually be on the end of this line

Thanks for the feedback! We've fixed the spacing issue with "if" and moved the curly brace. We've also checked and adjusted similar formatting issues throughout.

rmackay9 · 2024-10-30T10:14:08Z

libraries/AP_Mount/AP_Mount_Tusuav.cpp

+            if(mnt_target.angle_rad.yaw_is_ef){
+                gimbal_angle_mode.mode.bits.yaw = static_cast<uint8_t>(GimbalAngleMode::Earth_Frame_Angle);
+                debug("euler：%f  %f", gimbal_angle_mode.ef_angle[0], gimbal_angle_mode.ef_angle[2]);
+            }


little white space issue here. The else should be on the same line as the previous curly bracket

We've fixed this whitespace issue and reviewed similar formatting across the code to ensure consistency. Thank you!

rmackay9 · 2024-10-30T10:15:32Z

libraries/AP_Mount/AP_Mount_Tusuav.cpp

+                }
+
+                case GimbalSubCmd::GimbalSubCmd_LaserDistance:{
+					// recieve laser rangefinder value in decimeters and save the value in meters


little white space issue here (too much indenting)

Thanks for pointing that out! We've adjusted the indentation here and reviewed similar formatting throughout.

rmackay9 · 2024-10-30T10:15:54Z

libraries/AP_Mount/AP_Mount_Tusuav.cpp

+                    }
+                    break;
+                }
+                default:break;


we normally separate these into two lines

Thank you for your reply. I will make modifications based on your suggestions. Another question, can I continue submitting the modified code? Do I need to create a new pull request?

Hi @sunny-1987,

feel free to keep updating this PR. No need to create a new one. By the way, if you think we're going to slow in reviewing, the best thing to do is post a comment asking that the PR be added to the weekly dev call. You can even attend if you like it is 8am Japan time each tuesday. If that's too early then there's an "EU Dev Call" on Wed at 4pm (Japan Time).

Hi! Could you let me know how to join the "EU Dev Call" on Wednesdays? Thanks!

Hi @sunny-1987, just join ArduPilot's discord server and then go to the "weekly-devcall" voice channel at 4pm Japan time today.

What is your discord handle @sunny-1987 ? I'd like to try to contact you while I'm in China. I'm "timtuxworth" on WeChat/WeiXin.

rmackay9 · 2024-10-30T10:20:32Z

Thanks very much for this. In general it looks like it's well written.

It might be good to send me a gimbal so that I can properly test it and confirm that all the features are working. That's not completely necessary though, so no pressure.

By the way, I see there is a "merge" commit in there which we normally don't merge. It would be good to get rid of this by rebasing on master.

andyp1per · 2024-10-30T12:13:02Z

libraries/AP_Mount/AP_Mount_config.h

@@ -60,6 +60,10 @@
 #define HAL_MOUNT_TOPOTEK_ENABLED AP_MOUNT_BACKEND_DEFAULT_ENABLED
 #endif

+#ifndef HAL_MOUNT_TUSUAV_ENABLED
+#define HAL_MOUNT_TUSUAV_ENABLED AP_MOUNT_BACKEND_DEFAULT_ENABLED


This is a big chunk of code, enabled by default probably needs to be a bit more nuanced

I agree, the flash cost will likely be the biggest issue in getting this merged. There are certainly things we can do to share more code amonst the various gimbals but whether we need to do that before this gets merged or not is a good question

file.zip
We evaluated the flash storage conditions before and after implementing AP_Mount_Tusuav, and the results are attached. Additionally, we have included a video demonstrating our gimbal functionality, and we are using the CubeOrangePlus flight controller.

fyi, we have several CI tests below that check sizes for several classes of autopilot

peterbarker · 2024-11-08T23:19:37Z

libraries/AP_Mount/AP_Mount_Tusuav.cpp

+#include "AP_Mount_Tusuav.h"
+#if HAL_MOUNT_TUSUAV_ENABLED


Suggested change

#include "AP_Mount_Tusuav.h"

#if HAL_MOUNT_TUSUAV_ENABLED

#include "AP_Mount_config.h"

#if HAL_MOUNT_TUSUAV_ENABLED

#include "AP_Mount_Tusuav.h"

peterbarker · 2024-11-09T00:01:38Z

libraries/AP_Mount/AP_Mount_Tusuav.h

+#include "AP_Mount_Backend_Serial.h"
+
+#if HAL_MOUNT_TUSUAV_ENABLED


Suggested change

#include "AP_Mount_Backend_Serial.h"

#if HAL_MOUNT_TUSUAV_ENABLED

#include "AP_Mount_config.h"

#if HAL_MOUNT_TUSUAV_ENABLED

#include "AP_Mount_Backend_Serial.h"

peterbarker · 2024-11-09T00:02:06Z

libraries/AP_Mount/AP_Mount_Tusuav.h

+    // // init - performs any required initialisation for this instance
+    // void init() override;
+


Suggested change

// // init - performs any required initialisation for this instance

// void init() override;

peterbarker · 2024-11-09T00:02:36Z

libraries/AP_Mount/AP_Mount_Tusuav.h

+
+    // get attitude as a quaternion.  returns true on success
+    bool get_attitude_quaternion(Quaternion& att_quat) override;
+private:


Suggested change

private:

private:

peterbarker · 2024-11-09T00:04:46Z

libraries/AP_Mount/AP_Mount_Tusuav.h

+    enum class GimbalSubCmd : uint8_t {
+        GimbalSubCmd_GimbalAngle           =   0x16,   // retrieve gimbal angles
+        GimbalSubCmd_PlaneSensor_Attitude  =   0x18,   // send plane sensor & attitude to gimbal


Seems to be some oddity in naming here.

Perhaps this enum should just be Cmd?

Or strip some component out of the entries?

Thank you for your comments! The GimbalSubCmd enum represents sub-commands linked to MasCmd_Gimbal = 0xBD in the GimbalCmd enum. It defines specific operations for controlling and retrieving gimbal status, including angle retrieval, sensor attitude synchronization, parameter adjustments, laser measurement retrieval, and zoom or tracking configurations. We have reviewed the GimbalSubCmd entries and confirmed that each one is essential for its respective function.

peterbarker · 2024-11-09T00:46:34Z

libraries/AP_Mount/AP_Mount_Tusuav.cpp

+        return false;
+    }
+
+    int32_t int_zoom_value = (int32_t)zoom_value;


Suggested change

int32_t int_zoom_value = (int32_t)zoom_value;

const int32_t int_zoom_value = (int32_t)zoom_value;

... and in general just const everything that can be in here

Thank you for the suggestion. We’ve updated the code to apply 'const' where applicable.

peterbarker · 2024-11-09T00:46:52Z

libraries/AP_Mount/AP_Mount_Tusuav.cpp

+
+    int32_t int_zoom_value = (int32_t)zoom_value;
+    int16_t speed = AP_MOUNT_TUSUAV_ZOOM_SPEED;
+    if (zoom_type == ZoomType::RATE) {


Thank you！

peterbarker · 2024-11-09T00:47:50Z

libraries/AP_Mount/AP_Mount_Tusuav.cpp

+    }
+
+    default:{
+        return 0;


Suggested change

return 0;

return false;

peterbarker · 2024-11-09T00:48:19Z

libraries/AP_Mount/AP_Mount_Tusuav.cpp

+        return false;
+    }
+
+    MessageCameraSource camera = {


Suggested change

MessageCameraSource camera = {

const MessageCameraSource camera{

... any many other places

Ok,thank you！

peterbarker · 2024-11-09T00:48:32Z

libraries/AP_Mount/AP_Mount_Tusuav.cpp

+    };
+
+    FrameType frame_to_send{
+    .flag={0},


Thank you！

All reviews have been addressed.

All reviews have been addressed. 11.15

Hwurzburg added the WikiNeeded needs wiki update label Oct 29, 2024

rmackay9 reviewed Oct 30, 2024

View reviewed changes

sunny-1987 force-pushed the master branch 2 times, most recently from 255b1d9 to ba1a6df Compare October 30, 2024 11:55

andyp1per reviewed Oct 30, 2024

View reviewed changes

peterbarker requested changes Nov 9, 2024

View reviewed changes

sunny-1987 requested review from timtuxworth and rmackay9 November 15, 2024 04:15

sunny-1987 force-pushed the master branch from d57f59e to a5b4e3a Compare November 15, 2024 08:25

yi and others added 6 commits November 15, 2024 16:38

AP Mount: add Tusuav backend

0eeb323

AP Mount: integrate Tusuav gimbal driver

d7ad2af

Tools: add build option for Tusuav mount

7f85458

AP_Mount: Update Tusuav backend

01a0b1b

All reviews have been addressed.

AP_Mount: Update Tusuav backend 11.15

4c52c25

All reviews have been addressed. 11.15

AP_Mount: Update Tusuav backend 11.15

1176164

All reviews have been addressed. 11.15

sunny-1987 force-pushed the master branch from 3067675 to 1176164 Compare November 15, 2024 08:39

yi added 2 commits November 15, 2024 17:50

AP_Mount: Update Tusuav backend 11.15_2

f6a311d

AP_Mount: Update Tusuav backend 11.15_3

d479b1f

		uint32_t now_ms = AP_HAL::millis();


		if(now_ms - _last_update_ms_parse >= AP_MOUNT_TUSUAV_PARSE_INTERVAL_MS)

		#include "AP_Mount_Backend_Serial.h"

		#if HAL_MOUNT_TUSUAV_ENABLED

		// // init - performs any required initialisation for this instance
		// void init() override;

	int32_t int_zoom_value = (int32_t)zoom_value;
	const int32_t int_zoom_value = (int32_t)zoom_value;

	MessageCameraSource camera = {
	const MessageCameraSource camera{

Integrated Tusuav's gimbal driver and backend #28491

Are you sure you want to change the base?

Integrated Tusuav's gimbal driver and backend #28491

Conversation

sunny-1987 commented Oct 29, 2024

Tusuav Gimbal

The protocol description

The website of Tusuav

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rmackay9 Oct 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timtuxworth Nov 15, 2024 • edited Loading

Choose a reason for hiding this comment

rmackay9 commented Oct 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rmackay9 Oct 30, 2024 •

edited

Loading

timtuxworth Nov 15, 2024 •

edited

Loading