AP_SmartAudio: initial implementation #11630

SergeyBokhantsev · 2019-06-21T06:56:38Z

No description provided.

sh83 · 2019-06-21T07:54:28Z

This is probably the most asked feature on rcgroups forum.

libraries/AP_SmartAudio/AP_SmartAudio.cpp

khancyr · 2019-06-21T08:43:42Z

hello,

Do you have some link on documentation or what is smartaudio ? (I try googling but they is to much flood on the subject)

SergeyBokhantsev · 2019-06-21T08:50:22Z

This is about controlling a video tx with serial port
http://www.team-blacksheep.com/tbs_smartaudio_rev09.pdf

https://discuss.ardupilot.org/t/is-there-a-way-to-get-smartaudio-working/29025/20

ArduCopter/config.h

libraries/AP_SmartAudio/AP_SmartAudioDevice.h

libraries/AP_SmartAudio/AP_SmartAudioDeviceV2_OneWay.h

libraries/AP_SmartAudio/AP_SmartAudioDevice.cpp

libraries/AP_SmartAudio/AP_SmartAudio.h

libraries/AP_SmartAudio/AP_SmartAudio.cpp

OXINARF

@SergeyBokhantsev Let me start by thanking you for this contribution, this has been requested multiple times.

However, there are a couple of issues with it:

the commits need to follow our rules (which I think you are aware of), namely the first commit should be split into multiple commits, one per vehicle/library
I think this is way more complicated than it should be:
- the various versions of the protocol are very similar so having multiple classes doesn't look necessary
- the protocol version can be obtained automatically, no need to have the users set it
- I don't quite understand the "one way" versions, the replies are the confirmation that the command took effect; even if we didn't use them, I don't see any good reason to have a special version just for it
- you have implemented v2 which only has 4 power levels (from 0 to 3), but there are parameters for setting 5 power levels and setting them isn't that useful since they'll be 0 to 3... I understand that v1 and v2.1 can set multiple values (instead of just the 4 levels), but we can address that if we ever get those implemented

If you need help or want to discuss my concerns above, I'm usually around
Gitter chat too (http://gitter.im/ArduPilot/ardupilot).

OXINARF · 2019-06-25T13:50:31Z

libraries/AP_SmartAudio/AP_SmartAudio.cpp

+
+    // @Param: PROTOCOL
+    // @DisplayName: SmartAudio protocol version
+    // @Description: 0: None, 3: Version 2.0 One way


From the link you posted, looks like there are 3 versions: v1, v2, v2.1. What is the One way version?

Also, why isn't this automatically detected? The protocol provides it from the Get Settings command.

@OXINARF thanks for the such a complete review! I will go through the questions soon but want to clarify about OneWay and autodetect. The problem is SmartAudio physically is a single-wire half-duplex protocol. That means you have options:

To somehow configure your MCU to be a half duplex. I don't know if this possible for a wide range of the supported FC

To connect SA line of VTX to TX/RX lines on FC using the resistors scheme. I personally didn't tested this yet.

To connect SA line of VTX to TX line on FC, that's why I calling that OneWay. You're not able to get a response but for 90% of the use cases it is not needed :)

To somehow configure your MCU to be a half duplex. I don't know if this possible for a wide range of the supported FC

I'm not sure if you are aware that we do support half-duplex, you just have to set the appropriate SERIALx_OPTIONS ibt. According to code comments, it's supported in all F4 and F7s, so quite a wide range should support it.

To connect SA line of VTX to TX line on FC, that's why I calling that OneWay. You're not able to get a response but for 90% of the use cases it is not needed :)

Ok, if we want to support that then we can have 4 options:

0: auto-detect

1: v1

2: v2

3: v2.1

Then we can rate limit the sending if needed (I think it's quite low rate already but I'm not against further limiting).

@OXINARF No, I wasn't aware F3 supports half-duplex. I found the half-duplex option goes to UART implementation, but is there any example or doc about using that mode? Or there is no difference and I can just use .Read() and .Write() as in normal mode?

No, I wasn't aware F3 supports half-duplex.

Did you meant F4? Because we don't support F3.

I found the half-duplex option goes to UART implementation, but is there any example or doc about using that mode? Or there is no difference and I can just use .Read() and .Write() as in normal mode?

There is nothing different, usage is the same.

Yes, I meant F4. Ok thanks, will try that.

OXINARF · 2019-06-25T13:55:00Z

libraries/AP_SmartAudio/AP_SmartAudio.cpp

+
+    // @Param: TRAILZERO
+    // @DisplayName: Enable trailing zero
+    // @Description: If enabled the trailing zero will be sent after SA frame. Some VTX requires that.


Interesting. Would it be possible to always send this? Looks like a hard parameter for users to figure out.

OXINARF · 2019-06-25T13:58:43Z

libraries/AP_SmartAudio/AP_SmartAudio.cpp

+
+    // @Param: PWR_MODE
+    // @DisplayName: Power mode 
+    // @Description: 0-4: Using of PWR_0 - PWR_4 value, 5: Use Auto power mode based on home distanse 


This would be better as a @Values (with 0 to 4 just saying Power 0, etc.). It should still have a description though.

OXINARF · 2019-06-25T13:58:59Z

libraries/AP_SmartAudio/AP_SmartAudio.cpp

+    // @DisplayName: Power mode 
+    // @Description: 0-4: Using of PWR_0 - PWR_4 value, 5: Use Auto power mode based on home distanse 
+    // @User: Standard
+    AP_GROUPINFO("PWR_MODE", 4, AP_SmartAudio, power_mode, 0),


Should this really be 0 by default? I would expect Auto to be preferred.

OXINARF · 2019-06-25T14:02:54Z

libraries/AP_SmartAudio/AP_SmartAudio.cpp

+
+    // @Param: PWR_0
+    // @DisplayName: Power #0
+    // @Description: First (minimal) VTX power level


It doesn't need to be minimal, right?

OXINARF · 2019-06-25T16:09:26Z

libraries/AP_SmartAudio/AP_SmartAudioDevice.h

@@ -0,0 +1,69 @@
+#pragma once
+
+#include <AP_Common/AP_Common.h>


What is this needed for?

OXINARF · 2019-06-26T09:26:06Z

libraries/AP_SmartAudio/AP_SmartAudioDeviceV2_OneWay.cpp

+
+    send_command(cmd, sizeof(cmd));
+
+    actual_state.power = desired_state.power;


This should actually be done when getting the answer (which you are ignoring).

OXINARF · 2019-06-26T09:46:41Z

libraries/RC_Channel/RC_Channel.h

@@ -170,7 +170,8 @@ class RC_Channel {
        ALTHOLD   =           70, // althold mode
        FLOWHOLD  =           71, // flowhold mode
        CIRCLE    =           72, // circle mode
-        DRIFT     =           73  // drift mode
+        DRIFT     =           73, // drift mode
+        SA_PWR_LOW_OR_AUTO =  74, // SmartAudio VTX power toggle: low aux for minimal power, high aux for auto power


I think this should be PWR0_OR_AUTO. As written in a comment above, it doesn't really need to be a low power.

This needs to be added to parameter documentation.

OXINARF · 2019-06-26T10:02:07Z

libraries/AP_SmartAudio/AP_SmartAudio.cpp

@@ -101,15 +101,9 @@ void AP_SmartAudio::update()
            // TODO: calculate power basing on home distance


Are you going to add this in this PR? Otherwise we should really remove everything about it, we don't want dead code.

OXINARF · 2019-06-26T10:04:25Z

libraries/AP_SmartAudio/AP_SmartAudio.cpp

-
-        case SmartAudio_Power_4: power = power_4; break;
+        default:
+            power = power_value[power_mode];


You need to check that power_mode is valid, otherwise you could be doing an out of array memory access.

Seeelefant · 2019-10-26T20:36:01Z

What´s the current status? I am also highly interested in this functionality 🥇

Seeelefant · 2019-10-27T10:42:50Z

What`s the current status of this feature? I am also highly interested 👍

manotroll · 2019-10-28T01:42:56Z

Any way to test the current support?

SergeyBokhantsev · 2020-02-03T14:53:24Z

This PR should be closed because of implementation overhead, also note @OXINARF review notes that shall be taken into account.
I did another attempt https://github.com/SergeyBokhantsev/ardupilot/commits/SmartAudio2 but I have no time to complete it, unfortunately.
So anyone interested are welcome to use this branch as a base.
Personally I believe at moment that ArduPilot scripting is the best approach for that protocols.

Seeelefant · 2020-10-10T14:21:17Z

Any news on this topics? In which release can we expect the Smart Audio feature?

vierfuffzig · 2020-10-11T00:13:18Z

this has been stale for while. see #15174 for reference on current affairs.

Seeelefant · 2020-10-11T11:51:50Z

Ok, unfortunate, but hopefully @andyp1per will succeed this time :-)

khancyr · 2021-03-19T10:47:30Z

smartaudio support was bring by #16464. Thanks for the begin the effort on this !

AP_SmartAudio: initial implementation

ba8bb60

sh83 added AllVehicles Enhancement Library labels Jun 21, 2019

sh83 reviewed Jun 21, 2019

View reviewed changes

libraries/AP_SmartAudio/AP_SmartAudio.cpp Show resolved Hide resolved

sh83 reviewed Jun 21, 2019

View reviewed changes

libraries/AP_SmartAudio/AP_SmartAudio.cpp Show resolved Hide resolved

sh83 reviewed Jun 21, 2019

View reviewed changes

ArduCopter/config.h Outdated Show resolved Hide resolved

muramura reviewed Jun 21, 2019

View reviewed changes

AP_SmartAudio: PR review fixes

3b32a36

sh83 requested a review from WickedShell June 22, 2019 04:11

OXINARF requested changes Jun 26, 2019

View reviewed changes

OXINARF added Copter NeedRework and removed AllVehicles labels Jun 26, 2019

tridge self-assigned this Oct 26, 2019

ivanplkolev approved these changes Feb 2, 2020

View reviewed changes

This was referenced May 16, 2020

SmartAudio #14377

Closed

SmartAudio protocol #14383

Closed

khancyr closed this Mar 19, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AP_SmartAudio: initial implementation #11630

AP_SmartAudio: initial implementation #11630

SergeyBokhantsev commented Jun 21, 2019

sh83 commented Jun 21, 2019

khancyr commented Jun 21, 2019 •

edited

SergeyBokhantsev commented Jun 21, 2019 •

edited

OXINARF left a comment

OXINARF Jun 25, 2019

SergeyBokhantsev Jun 26, 2019

OXINARF Jul 4, 2019

SergeyBokhantsev Jul 26, 2019

OXINARF Jul 29, 2019

SergeyBokhantsev Jul 30, 2019

OXINARF Jun 25, 2019

OXINARF Jun 25, 2019

OXINARF Jun 25, 2019

OXINARF Jun 25, 2019

OXINARF Jun 25, 2019

OXINARF Jun 26, 2019

OXINARF Jun 26, 2019

OXINARF Jun 26, 2019

OXINARF Jun 26, 2019

Seeelefant commented Oct 26, 2019

Seeelefant commented Oct 27, 2019

manotroll commented Oct 28, 2019

SergeyBokhantsev commented Feb 3, 2020

Seeelefant commented Oct 10, 2020

vierfuffzig commented Oct 11, 2020

Seeelefant commented Oct 11, 2020

khancyr commented Mar 19, 2021

		@@ -0,0 +1,69 @@
		#pragma once

		#include <AP_Common/AP_Common.h>


		send_command(cmd, sizeof(cmd));

		actual_state.power = desired_state.power;

		@@ -101,15 +101,9 @@ void AP_SmartAudio::update()
		// TODO: calculate power basing on home distance

AP_SmartAudio: initial implementation #11630

AP_SmartAudio: initial implementation #11630

Conversation

SergeyBokhantsev commented Jun 21, 2019

sh83 commented Jun 21, 2019

khancyr commented Jun 21, 2019 • edited

SergeyBokhantsev commented Jun 21, 2019 • edited

OXINARF left a 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

Seeelefant commented Oct 26, 2019

Seeelefant commented Oct 27, 2019

manotroll commented Oct 28, 2019

SergeyBokhantsev commented Feb 3, 2020

Seeelefant commented Oct 10, 2020

vierfuffzig commented Oct 11, 2020

Seeelefant commented Oct 11, 2020

khancyr commented Mar 19, 2021

khancyr commented Jun 21, 2019 •

edited

SergeyBokhantsev commented Jun 21, 2019 •

edited