Skip to content
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.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automatic telemetry modes Standard and NoTelemOnArm #1599

Merged
merged 17 commits into from Jun 7, 2022

Conversation

CapnBry
Copy link
Member

@CapnBry CapnBry commented May 29, 2022

Adds two new dynamic telemetry modes options:

  • Standard - Std which changes the telemetry ratio dynamically based on our recommendation.
  • Disarmed - Race same as Standard, but turns telemetry OFF when armed and also implies NO_SYNC_ON_ARM

Standard

Standard telem ratio uses the Packet Rate to maintain approximately 4x telemetry packets per second for classic rates, or 3x packets per second for fullres rates. This has been changed to the default ratio, since newbies won't know they should always be changing their TLM Ratio every time they change the packet rate.

Disarmed

This is where the real magic happens and I am sort of pleased at the elegance of my solution. Racers have a bit of a conundrum in that they want to use VTX Administrator, but it only auto-updates the VTX settings when the connection is established, which requires telemetry. However, when racing telemetry is wasted packets and is not needed, forcing them to manually have to Send VTX to change settings. With TLM set to Disarmed, when disarmed, the TX requests telemetry at the Standard rate, which means there is a connection event on a new plugin allowing the VTX settings to be sent, and when arming the telemetry is disabled automatically. Disarming turns telemetry back on automatically as well.

I've highlighted the uploading of VTX settings in green here
image

Racers also despise wasting packets on SYNC packets and therefore we have the NO_SYNC_ON_ARM define to disable them. I've grouped this functionality with TLM=Disarmed since it is the same purpose.

TLM=Disarmed works reliably

Note the pink highlighted sync packets in the scoreboard above. When the pilot arms the sync packet starts informing the RX it wants telemetry off, but it doesn't know for sure that that single sync packet was received by the RX. To guarantee this, the TX keeps listening at the old telemetry interval, highlighted in yellow as missing packets at the RX. The RX in the example received the first TLM=Off sync packet, but the TX will keep sending TLM=Off and listening until it misses 5x telemetry packets in a row and goes into the disconnected state. At that point, a final sync packet is sent quickly (low disconnected sync interval) and the TX stops sending sync packets after that. The whole process takes at most ~6s, or around 3s on average, assuming the RX receives the first sync. However, it will eventually always manage to disable telemetry on the RX and go to 100% RC Channels packets.

Reenabling telemetry on disarm is even quicker. Because the TX is in a disconnected state, when the user disarms the TX will send sync packets the never next time it is on the sync channel after disarm, trying to reestablish the connection. Note that this will of course reupload the VTX Settings due to it being a new connection as seen in the scoreboard.

Mode Names

The names displayed in the Lua are short for two reasons.

  • For these two modes the effective TLM Ratio is shown after the mode name. On B&W screens that means only 3 or 4 characters can fit. Race is maybe a little cryptic but !Armed and Disarmed are too long.
  • Shorter options have less Lua memory overhead! Lord help us of we have another Pitmode or DVR AUX on our hands. 馃槢

I'm open to a better name than Race since it describes a usage, not what the mode does. Alternatives: dArm, !Arm. It isn't too important that it be obvious to new users, it isn't an option a new user would select anyway and we can't describe its function even in 10 characters.

Std might also cause a little confusion but Auto is misleading because we don't adjust it based on the amount of data, as it implies. Basic implies it doesn't do something, like you need to set the ratio to get advanced telemetry or something.

Unrelated Change

Building the dynamic folder names / units labels takes a lot of code space, so I locked TxPower and VtxAdmin to building their strings at a fixed point. Half the code already did this so I don't see why that one should be dynamic even though it is operating against a fixed string. Saves 176 bytes.

@pkendall64
Copy link
Collaborator

pkendall64 commented May 29, 2022

I like this, simplify for the mere mortals but allow them racer bois to get every single packet through.
I think !Arm and Std are good. The documentation could describe Std as maintaining a constant number of telemetry packets per second.

EDIT: Actually I quite like Race as it describes the intent of that telemetry mode.

@CapnBry
Copy link
Member Author

CapnBry commented May 30, 2022

I've shaved another 20 bytes off the code, bringing this change down to 168 bytes of code total, and I think that's about the best I can do. Quite a bit of that is building the dynamic units field for the Lua, so I cut some [unrelated] code used for building the labels and brought it down to a total of -8 bytes used to create this feature!

@froqstar
Copy link

newbies won't know they should always be changing their TLM Ratio every time they change the packet rate.

What if we set telemetry rate in Hz (2Hz, 4Hz, 8Hz etc), and automatically calculate the telemetry ratio from it?
Saves mental load having to calculate manually every time, and removes the need to change it whenever changing packet rate.

@CapnBry
Copy link
Member Author

CapnBry commented May 30, 2022

What if we set telemetry rate in Hz (2Hz, 4Hz, 8Hz etc), and automatically calculate the telemetry ratio from it?

That's just more confusing options, a per-second rate correlates to nothing to the user. "8Hz" is around 8 packets per second but that's not 8 telemetry items, or updating all the telemetry items 8x a second either. The numbers don't really work out very evenly either, since even our hand-picked 4Hz values end up being anywhere from 3.125Hz to 4.68745Hz.

This PR is more aimed toward reducing the burden of learning the system and preventing new users from ending up with not enough telemetry when they turn down the packet rate, but leave the Ratio 1:128. That, and also allow that behavior to be applied to the Disarmed telemetry mode.

@Roman-Golod
Copy link

I'm open to a better name than Race since it describes a usage, not what the mode does.
If strikethrough font can be used, maybe use
Arm with it?

@CapnBry CapnBry added the needs doc 馃摉 Tagged to remember to create docs for this label Jun 4, 2022
@deadbytefpv
Copy link
Contributor

deadbytefpv commented Jun 5, 2022

I'm not so sure if I'm testing it wrong, but only 1:64 and above is getting me a Telemetry sync (and the C on the top right corner of the Lua Script).
Std, 1:128 and Race aren't giving me Telemetry frames (not Armed)
The display on the OLED (testing using a Jumper Aion Nano TX, with a betaFPV Nano 2.4GHz receiver) doesn't match up with the selected TLM Ratio as well (Off by one index; 1:64 appears as 1:32, etc)

testing on d65b4bc

https://streamable.com/d4fttv

@CapnBry
Copy link
Member Author

CapnBry commented Jun 5, 2022

Deadbyte was right about the OLED (I forgot) and the TLM being off by one at the RX. Haha I was so clever I thought, well the OTA is 100% compatible so no changes to the RX code. Well, only if the RX doesn't know that there are other rates! Fixed both of these.

The OTA is the same, 0=NoTLM, 1=1:128, ... 7=1:2, so spi-dev doesn't need to change anything for this, so long as they assume the effective TLM ratios are still encoded the same way.

@deadbytefpv
Copy link
Contributor

Tested the latest commit 417d601
and all seems to be working including the sync on 1:128, Std and Race.. C on top right corner of Lua script appears, and receiver LED is solid.
Also tested Race mode: arming and losing Telemetry a few seconds after.

OLED also matches currently selected TLM Ratio, including Std and Race.

Tested on the Jumper AION Nano TX and a BetaFPV Nano 2.4GHz.

src/src/tx_main.cpp Outdated Show resolved Hide resolved
@JyeSmith
Copy link
Member

JyeSmith commented Jun 7, 2022

Ive been waiting for this feature... soooo nice to have 馃挭

Copy link
Collaborator

@pkendall64 pkendall64 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another great PR with nothing to complain about!

@JyeSmith JyeSmith merged commit 7227614 into ExpressLRS:master Jun 7, 2022
bkleiner pushed a commit to bkleiner/ExpressLRS that referenced this pull request Jun 12, 2022
* Add new default Standard telemetry rate

* Prevent any OTA changes by putting Std last

* Add default ratios for all air rates

* Add TLM_RATIO_DISARMED

* Remove NO_SYNC_ON_ARM

* Shave 16 bytes off code and move Std to first item

* Another 4 bytes, disconnect is implied

* Lock start of TxPower/VTXAdmin dynamic folder name

* Rename !Arm to Race

* Missed some sync_on_arm

* Remove no-sync-on-arm again haha it will not die

* Fix new TLM modes missing on display

* Fix RX TLM ratio off by 1 (OTA is compatible, code is not)

* TLM_DISARMED always uses disconnected sync interval
@CapnBry CapnBry deleted the telem-for-dummies branch June 23, 2022 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs doc 馃摉 Tagged to remember to create docs for this V3.0 馃殌
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants