Add TBS SmartAudio support #1283

jflyper · 2016-10-08T15:47:13Z

This is the rebased version of #1275 (onto development branch).
It now works with the new OSD code on OMNIBUS (F3) target.

For this cut, target.h related configuration options are:

VTX_SMARTAUDIO Turns on SmartAudio support code

VTX_CONTROL Turns on generic task that listens to VTX that talks. Calls SmartAudio specific task function for now, but likely to be expanded to call IRC Tramp specific task in the near future.
.
The PR only supports OMNIBUS F3 as of now, since applicability and configuration is extremely limited at the moment; a target must support OSD (bfosd) option, and don't support USE_RTC6705, of course (that means non-SIRINFPV target, no idea about RACEBASE target).

~~To test the PR, SmartAudio signal goes to UART2 TX (hardcoded in drivers/vtx_smartaudio.c). UART1 could be used, but requires to mod the hardcode.~~

UART is now configurable with cli serial command; use mask value 1024.

EDIT
The PR was prepared for v3.0.0 master, but due to recent big change in io/osd.c, it is not merge-able. Anyone willing to test should fetch the whole branch: https://github.com/jflyper/cleanflight/tree/bf300-smartaudio

Works with the new OSD code; full band/chan control, limited power control (25 & 200mW).

EDIT2
The PR includes #1269
.
EDIT3
RACEBASE target may be modified to support the smartaudio: add VTX_SMARTAUDIO and VTX_CONTROL.

EDIT
Discussion (issue) thread is at #1029

Required for SmartAudio Support with single wire connection and without external pull-up. Tested with F3 only. F1 and F4 has untested mods, and Softserial haven’t been touched.

martinbudden · 2016-10-09T18:08:26Z

src/main/drivers/vtx_smartaudio.c

+#include "drivers/vtx_smartaudio.h"
+#include "io/serial.h"
+#include "fc/runtime_config.h"
+#include "config/config_master.h"


Drivers should not call upwards, so should not include any of:
io/serial.h
fc/runtime_config.h
config/config_master.h

I think I can live without fc/runtime_config.h and config/config_master.h.

io/serial.h is included for

To find out a port assigned to the smartaudio.

To open a uart port for printf() debugging.

2 will eventually go away, but where should I put 1? main.c?

The thing that calls smartAudioInit should open the port and pass it in as a parameter to the smartAudioInit function. So yes, main.c. [We might elect to change that in future, but for the moment that is the correct place.]

ACK
Thanks for the suggestion!

I'm beginning to think about moving this to io since it never touches raw I/O directly, or separating it into a driver that handles up to transport layer (framing + retransmission) and a module in io that handles the vtx control protocol. Thoughts?

Yes, you're right - none of this code should be in the driver, I should have spotted that.

So I'd move all code into the IO layer. However the comments about reducing interdependencies still apply - I would still make the changes suggested above.

martinbudden · 2016-10-09T18:10:35Z

src/main/drivers/vtx_smartaudio.c

+
+void smartAudioProcess(uint32_t now)
+{
+    bool armedState = ARMING_FLAG(ARMED) ? true : false;


Driver should not have knowledge of arming. Instead just don't call this function if armed.

martinbudden · 2016-10-09T18:11:07Z

src/main/fc/mw.c

+void taskVtxControl(uint32_t currentTime)
+{
+#ifdef VTX_SMARTAUDIO
+    smartAudioProcess(currentTime);


Only call this function if armed.

martinbudden · 2016-10-10T07:19:01Z

I've had a bit of time to look at the code overall. Not much time, so this is only a fairly cursory review, but I have the following comments:

The use of a state machine to read the input frame seems like overkill. It's just a simple TLV frame with CRC
Likewise, the use of a queue send the output frame seems overkill.
There is an extended API under a #define. If no one uses this, then get rid of it - APIs should not be implemented without something that uses them. If it is used, then get rid of the #define.
There is quite a bit of debug code. If this has served its purpose, then get rid of it.

martinbudden · 2016-10-10T07:27:35Z

Also can you put a link to the SMARTAUDIO spec in this PR.

jflyper · 2016-10-10T13:05:54Z

@martinbudden

The use of a state machine to read the input frame seems like overkill. It's just a simple TLV frame with CRC

I believe we need a state machine based receive framer to handle variable length frame from unreliable input channel.

Likewise, the use of a queue send the output frame seems overkill.

This needs some explanation.

The smartaudio returns response for valid command frames in no less than 60msec, which we can't wait. So there's a need for a resend buffer.
Some completely valid user level single operation requires multiple commands to be sent.
For example, "Change RF power from PIT to 200mW" requires (1) setting RF power DAC to 200mW output then (2) cancel pit mode to make RF PA active.
Since we can't wait for the first operation to finish, we must queue commands. Here, I selected to queue it as ready to send command frames rather than commands and parameters as they are passed to each primitive functions; it also has a high affinity with the frame resend scheme.

There is an extended API under a #define. If no one uses this, then get rid of it - APIs should not be implemented without something that uses them. If it is used, then get rid of the #define.

I understand.
Most of them will be accessed by osd/config-menu shortly.

There is quite a bit of debug code. If this has served its purpose, then get rid of it.

Yeap. They will eventually be all gone.

martinbudden · 2016-10-10T15:08:45Z

@jflyper , you explanations make sense. Can you add some comments in the code to explain this? Otherwise, at some future date, someone might decide to "optimise" the code and remove the queue and state machine.

jflyper · 2016-10-24T13:16:22Z

Finishing #1352 first, so the smartaudio can be controlled through osd menu.

martinbudden · 2016-12-29T13:08:42Z

@jflyper , see my comments on the CMS code.

SmartAudio remembers everything by itself.

martinbudden · 2016-12-30T08:42:19Z

src/main/io/vtx_smartaudio_cms.h

@@ -0,0 +1 @@
+extern CMS_Menu cmsx_menuVtxSmartAudio;


I think you have missed this one. Shouldn't this be in cms directory?

For the smartaudio, corresponding CMS contents reside in io/vtx_smartaudio.c.

I first thought it would be nice to keep them together with objects and functions the are related with. However, I'm not so sure now, thinking about the possible consolidation of vtx related APIs, associated codes, operational model and of course, UIs. There is also a couple of new FC controllable vtx that we are expecting to join this game. So, my plan is to keep it as is for 3.1, and restructure it as a part of the vtx system reorganization later (3.2 or 3.3).

That makes sense.

jflyper · 2016-12-31T04:03:51Z

So, we have a problem in how to enable smartaudio support for all boards that potentially need it.

I was going to enable VTX_SMARTAUDIO and VTX_CONTRL in common.h, but realized that this will also enable smartaudio for targets with built-in vtx; targets with VTX (SINGULARITY and SPRACINGF3NEO) or USE_RTC6705 (SIRINFPV and FishDroneF4).

I am facing a similar problem with I2C configuration, in which common.h defines USE_DASHBOARD, but there are boards that does not enable any I2C. There, I used build/build_config.h to do post-target.h cleanup of conflicting defines. If I do the same here, then it will be something like the following.

common.h defines VTX_CONTROL and VTX_SMARTAUDIO.

#define VTX_CONTROL
#define VTX_SMARTAUDIO

Above mentioned targets define VTX or USE_RTC6705 in target.h, for example,

#define USE_RTC6705

build/build_config.h will sort these out.

#if defined(VTX) || defined(USE_RTC6705)
#  undef VTX_CONTROL
#  undef VTX_SMARTAUDIO
#endif

Other targets will retain VTX_CONTROL and VTX_SMARTAUDIO.

Any thoughts?

EDIT
Is it better to create an explicit configuration touch up header file?

…mon.h and target.h

stellarhopper · 2017-01-03T06:52:51Z

I tried to run this branch on the brainfpv fork:
(Here is my attempt at merging this PR into it)
https://github.com/stellarhopper/betaflight

I am able to get the menus on the OSD, and I see it tries to send packets at different baud rates, but it never receives a reply from the Unify (race edition, should be smartaudio v2.0)

I have the smartaudio pin wired to uart1, and the serial mask is set up correctly (to 2048), and the configurator built with the related pull request for this feature sees it correctly as peripherals/smartaudio.

The spec also says the unify baud rate could vary to +/- 5%, which for the nominal 4800, could mean anything between ~4500 to ~5100, and so I tried to increase the 'scanned' range min/max to those numbers, but still no response (the OSD does show it attempting the larger range).

Is there anything I could be missing?

jflyper · 2017-01-03T09:23:51Z

@stellarhopper
RE1 is F4, right?
I suspect the lack of SERIAL_BIDIR_PP option handling.

SERIAL_BIDIR turns on half-duplex, and IOCFG_AF_OD. It looks like SmartAudio doesn't like this, so I added SERIAL_BIDIR_PP option to set push-pull on half-duplex.

https://github.com/betaflight/betaflight/blob/master/src/main/drivers/serial_uart_stm32f4xx.c#L341-L347

    if (options & SERIAL_BIDIR) {
        IOInit(tx, OWNER_SERIAL_TX, RESOURCE_INDEX(device));
        if (options & SERIAL_BIDIR_PP)
            IOConfigGPIOAF(tx, IOCFG_AF_PP, uart->af);
        else
            IOConfigGPIOAF(tx, IOCFG_AF_OD, uart->af);
    }

https://github.com/stellarhopper/betaflight/blob/brainre1/src/main/drivers/serial_uart_stm32f4xx.c#L344-L357

    if (options & SERIAL_BIDIR) {
#ifdef USE_RE1_FPGA
        IOInit(tx, OWNER_SERIAL_TX, RESOURCE_INDEX(device));
        if (options & SERIAL_INVERTED) {
            IOConfigGPIOAF(tx, IOCFG_AF_PP_UP, uart->af);
        }
        else {
            IOConfigGPIOAF(tx, IOCFG_AF_OD, uart->af);
        }
#else
        IOInit(tx, OWNER_SERIAL_TX, RESOURCE_INDEX(device));
        IOConfigGPIOAF(tx, IOCFG_AF_OD, uart->af);
#endif
    }

stellarhopper · 2017-01-04T07:02:23Z

@jflyper Thanks! made that change, and it worked!
@mluessi I'm happy to submit a PR with this change once you pull in upstream betaflight next.

mluessi · 2017-01-04T15:16:28Z

@stellarhopper awesome. I'm in the process of updating it. Just push your changes to github and I will cherry pick your commit :).

Edit: found the branch on your repo, will take the changes from there :)

jflyper · 2017-01-04T15:30:01Z

@stellarhopper 🍻

Linjieqiang · 2017-01-04T17:39:50Z

@jflyper Great work!I want to know how to connect TBS SmartAudio to my F4 FC ?

SRWieZ · 2017-01-04T23:57:17Z

I tried it, it works great !

I have an OmibusF3 with TBS HV 5G8 Race Edition.

stellarhopper · 2017-01-13T08:48:47Z

@jflyper So I ran into what could possibly be a smartaudio related bug.

somehow I got into a mode where the unify would only power on in pitmode, and I'd have to take it out of pit mode manually on every boot, either by holding the button, or through the VTX menu if I was using PIR.

I was able to get it back out of this mode (always boot in pitmode) by holding the button while powering on, and then cycling through the 'menu' and finally saving (all on the vtx/button).

But I certainly didn't enable this mode myself by powering up while holding the button. So is there any way certain smartaudio commands could've been sent to put the vtx into this mode? (I haven't looked at the smartaudio spec to see if such a setting is even exposed..).

jflyper · 2017-01-13T09:01:09Z

@stellarhopper
TBH, I didn't do a in depth analysis of how pit mode interact with buttons and CMS menus. Their manual is too casual for me to establish a concrete picture of the interaction.

In terms of CMS for SmartAudio, yes, you can enable/disable the PIT mode:
PIT mode (unify powers up in pit mode) can be enabled by CMS FEATURES/VTX menu, CONFIG submenu and selecting RACE for OPMODEL (operational model). This change will take effect upon next power cycle of the Unify.

It would be grateful if you do some experiments regarding the interaction of buttons and the RACE/FREESTYLE selection.

stellarhopper · 2017-01-13T09:06:41Z

@jflyper yes, I've been doing experiments to try and trigger this again..
What is race vs freestyle supposed to mean... I was kind of under the assumption that RACE refers to the 'race' edition of the unify which only does 25mw and 200mw, and 'FREE' means the one that goes up to 800mw.. But sounds like
RACE = always boot in pitmode
vs
FREE = always boot with last selected power.

Is that correct?

It might be easier to understand the terminology if the OPMODE selection was instead
CLEANSWITCH <ON | OFF>
as that is the terminology used in the TBS manual.

I'm not sure how generic the menus are, but if they are Unify specific, then it might be worth making this change.

jflyper · 2017-01-13T09:11:24Z

But sounds like
RACE = always boot in pitmode
vs
FREE = always boot with last selected power.
Is that correct?

Yes. You are correct. I first named it PIT and FREE, but thought "PIT mode" is just a state or momentary mode, and whole operational scheme should called differently, thus the "RACE" operational model.

It might be easier to understand the terminology if the OPMODE selection was instead
CLEANSWITCH <ON | OFF>
as that is the terminology used in the TBS manual.

I didn't know the CLEANSWITCH stuff. Any pointers?

I'm not sure how generic the menus are, but if they are Unify specific, then it might be worth making this change.

I'm some what trying to make the concept of "operational model" generic :)

stellarhopper · 2017-01-13T09:20:35Z

For cleanswitch, grep for it in: http://www.team-blacksheep.com/tbs-unify-pro-5g8-manual.pdf
I guess this will be somewhat of a learning curve for users too..

I did just confirm that RACE == cleanswitch on, and FREE == cleanswitch off.
The button equivalent way of turning this mode on and off is holding the button while plugging in, this will start up the vtx in its 'menu', and then you go thhrough the menu stages (3s presses) till you save and exit out.

Unfortunately this doesn't seem to be documented in the manual at all, and I only know because TBS support guided me on how to do this..

I think the biggest point of confusion is probably caused by the 'RACE' wording, because that is also a model of the Unify, and people are assuming that is what it stands for, and enabling it :)

stellarhopper · 2017-01-13T09:37:39Z

In the least, I think there should be some safeguard against turning on both RACE and POR together.. (that is how I first hit the issue and feared that the unify had gotten fried or something, as it was transmitting nothing on any of the usual channels). Because this will always make it come up at the out of band freq at low power on every boot, and really I don't see a use case for this..

Someone made another suggestion, that the RACE/FREE selection could be made in a different 'screen' or submenu on the OSD, which will then have some space to describe both modes in some detail so users aren't completely caught off guard.

jflyper · 2017-01-14T04:54:54Z

@stellarhopper I see your points. For 3.1, I think I'm going to add an explanation of the status line to the wiki page for the smartaudio.

For the cleanswitch, I think it is a by-product of the pit mode...

ezshinoda added 6 commits October 8, 2016 23:23

SmartAudio Support

8334cd3

SERIAL_BIDIR_PP def in drivers/serial.h

f5dd171

SERIAL_BIDIR_PP option

c724b7c

Required for SmartAudio Support with single wire connection and without external pull-up. Tested with F3 only. F1 and F4 has untested mods, and Softserial haven’t been touched.

Removed rescheduling code and superfluous includes

d26a187

Rebasing onto development

a04d67c

Make UART configurable

3a58bbb

jflyper mentioned this pull request Oct 8, 2016

TBS SmartAudio support, first cut for v3.0.0 #1275

Closed

Make osd.c aware of SmartAudio power table

a3f9c3d

martinbudden reviewed Oct 9, 2016

View reviewed changes

Move everything to io

8c19d9f

ezshinoda added 2 commits October 14, 2016 01:36

wip

2813e5a

wip

d5eee59

martinbudden added the Rebase Required label Oct 15, 2016

wip

7cc1010

borisbstyle added the Testing Required label Oct 20, 2016

martinbudden changed the title ~~Add SmartAudio support~~ Add TBS SmartAudio support Oct 22, 2016

Pit mode first implementation

de74dbd

jflyper mentioned this pull request Oct 27, 2016

FuriousFPV Innova OSD-VTX support ShikOfTheRa/scarab-osd#327

Closed

borisbstyle closed this Oct 27, 2016

jflyper mentioned this pull request Oct 31, 2016

Bit-banged SPI based RTC6705 VTX support cleanflight/cleanflight#1948

Closed

ezshinoda added 3 commits November 1, 2016 11:57

wit (rebase onto master) blind merge

ab7bb13

Working version after rebase (onto master)

9264a9a

Rebased onto current CMS dev

2bf74e0

ezshinoda added 5 commits December 30, 2016 00:11

Merge remote-tracking branch 'betaflight/master' into bfdev-smartaudio

86bb650

Delete bogus files

9a4aaaa

Add copyright notice

9d6fb10

SmartAudio support to be included for >128K targets

e3826c1

Delete VTX_SMARTAUDIO from config_master.h

c8a8ee0

SmartAudio remembers everything by itself.

martinbudden reviewed Dec 30, 2016

View reviewed changes

martinbudden approved these changes Dec 30, 2016

View reviewed changes

martinbudden removed the Reviewing label Dec 30, 2016

ezshinoda added 2 commits December 31, 2016 14:30

Created target/common_post.h to touch up configuration created by com…

352d385

…mon.h and target.h

Merge remote-tracking branch 'betaflight/master' into bfdev-smartaudio

0b981ec

borisbstyle merged commit 3cc85e8 into betaflight:master Jan 3, 2017

jflyper mentioned this pull request Jan 14, 2017

TBS SmartAudio on OMNIBUSF4SD not working #2094

Closed

oleost mentioned this pull request Feb 9, 2017

Feature request, betaflight peripherals. Smartaudio and tramp vtx iNavFlight/inav#1262

Closed

aowi7280 mentioned this pull request Dec 18, 2018

GPS RTH issue: Max altitude doesn't reset with disarm #7243

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add TBS SmartAudio support #1283

Add TBS SmartAudio support #1283

jflyper commented Oct 8, 2016 •

edited

martinbudden Oct 9, 2016

jflyper Oct 9, 2016

martinbudden Oct 9, 2016

jflyper Oct 9, 2016 •

edited

jflyper Oct 10, 2016

martinbudden Oct 10, 2016

martinbudden Oct 9, 2016

jflyper Oct 9, 2016 •

edited

martinbudden Oct 9, 2016

jflyper Oct 9, 2016

martinbudden commented Oct 10, 2016

martinbudden commented Oct 10, 2016

jflyper commented Oct 10, 2016

martinbudden commented Oct 10, 2016

jflyper commented Oct 24, 2016

martinbudden commented Dec 29, 2016

martinbudden Dec 30, 2016

jflyper Dec 30, 2016

martinbudden Dec 30, 2016

jflyper commented Dec 31, 2016 •

edited

stellarhopper commented Jan 3, 2017

jflyper commented Jan 3, 2017

stellarhopper commented Jan 4, 2017

mluessi commented Jan 4, 2017 •

edited

jflyper commented Jan 4, 2017

Linjieqiang commented Jan 4, 2017

SRWieZ commented Jan 4, 2017

stellarhopper commented Jan 13, 2017

jflyper commented Jan 13, 2017

stellarhopper commented Jan 13, 2017

jflyper commented Jan 13, 2017

stellarhopper commented Jan 13, 2017

stellarhopper commented Jan 13, 2017

jflyper commented Jan 14, 2017 •

edited

Add TBS SmartAudio support #1283

Add TBS SmartAudio support #1283

Conversation

jflyper commented Oct 8, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jflyper Oct 9, 2016 • edited

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

jflyper Oct 9, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martinbudden commented Oct 10, 2016

martinbudden commented Oct 10, 2016

jflyper commented Oct 10, 2016

martinbudden commented Oct 10, 2016

jflyper commented Oct 24, 2016

martinbudden commented Dec 29, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jflyper commented Dec 31, 2016 • edited

stellarhopper commented Jan 3, 2017

jflyper commented Jan 3, 2017

stellarhopper commented Jan 4, 2017

mluessi commented Jan 4, 2017 • edited

jflyper commented Jan 4, 2017

Linjieqiang commented Jan 4, 2017

SRWieZ commented Jan 4, 2017

stellarhopper commented Jan 13, 2017

jflyper commented Jan 13, 2017

stellarhopper commented Jan 13, 2017

jflyper commented Jan 13, 2017

stellarhopper commented Jan 13, 2017

stellarhopper commented Jan 13, 2017

jflyper commented Jan 14, 2017 • edited

jflyper commented Oct 8, 2016 •

edited

jflyper Oct 9, 2016 •

edited

jflyper Oct 9, 2016 •

edited

jflyper commented Dec 31, 2016 •

edited

mluessi commented Jan 4, 2017 •

edited

jflyper commented Jan 14, 2017 •

edited