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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AP_LTM_Telem: Implementation of LTM telemetry protocol for Ardupilot #12158

Merged
merged 9 commits into from Nov 26, 2019

Conversation

@mariansoban
Copy link
Contributor

mariansoban commented Aug 27, 2019

Low bandwidth LTM protocol implementation based on code from artemen from thread: https://discuss.ardupilot.org/t/adding-ltm-telemetry-support/35385
Protocol overview: https://quadmeup.com/ltm-light-telemetry-protocol/
Note: Data speed is fixed to 2400 bauds.

Data sample from Logic Analyser: https://1drv.ms/u/s!AmxBUJd5HcNhh3D46wF66DZK3J63

Screenshot from Logic Analyser
logic analyser screenshot - G and A frames

Screen of Ghetto Antenna tracker:
ltm protocol at ghetto antenna tracker

EDIT (2nd Sept. 2019): More photos and screenshots added:

  1. Messages from Mission Planner - dev FW built from branch with PR
    01-mp-messages

  2. LTM serial setting @ MP
    02-mp-serial-settings
    Note: Current index of LTM protocol is 24, 23 is used by RC IN.

  3. Output of serial console from customized Ghetto Antenna Tracker, code: https://github.com/mariansoban/Ghettostation - contains many changes from original KipK's version, but LTM processing was kept intact
    03-station-serial-debug.txt

  4. Photo of testing setup - ArduPlane code is running on Pixhawk Lite, LTM data is sent to APM board where antenna tracker code is running
    04-testing-setup

  5. Detail photo of antenna tracker's display
    05-display-detail

Copy link
Contributor

peterbarker left a comment

Closes #3193

This looks pretty solid!

Commit messages need fixing; should have a single commit per logical directory, and the commit messages for each prefixed with the directory name. git log will show you how it looks.

libraries/AP_LTM_Telem/AP_LTM_Telem.cpp Outdated Show resolved Hide resolved
libraries/AP_LTM_Telem/AP_LTM_Telem.cpp Outdated Show resolved Hide resolved
libraries/AP_LTM_Telem/AP_LTM_Telem.cpp Outdated Show resolved Hide resolved
libraries/AP_LTM_Telem/AP_LTM_Telem.cpp Outdated Show resolved Hide resolved
libraries/AP_LTM_Telem/AP_LTM_Telem.h Outdated Show resolved Hide resolved
libraries/AP_LTM_Telem/AP_LTM_Telem.h Outdated Show resolved Hide resolved
libraries/AP_SerialManager/AP_SerialManager.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

peterbarker left a comment

Braces to enclose single expression.

libraries/AP_LTM_Telem/AP_LTM_Telem.cpp Outdated Show resolved Hide resolved
libraries/AP_LTM_Telem/AP_LTM_Telem.cpp Outdated Show resolved Hide resolved
@mariansoban mariansoban force-pushed the mariansoban:feature/ltm_protocol branch from 02bbc05 to 9dc5837 Aug 28, 2019
@peterbarker

This comment has been minimized.

Copy link
Contributor

peterbarker commented Aug 28, 2019

An acknowledgement or two of your sources would also seem to be in order.

Copy link
Contributor

peterbarker left a comment

As mentioned, probably not worth doing the static asserts.

libraries/AP_LTM_Telem/AP_LTM_Telem.cpp Show resolved Hide resolved
libraries/AP_LTM_Telem/AP_LTM_Telem.cpp Outdated Show resolved Hide resolved
@mariansoban mariansoban force-pushed the mariansoban:feature/ltm_protocol branch from 9dc5837 to f7f25d0 Aug 28, 2019
@mariansoban

This comment has been minimized.

Copy link
Contributor Author

mariansoban commented Aug 29, 2019

Is there any way how to re-run (force) travis-ci build? There was a timeout before, that caused build fail:

Error: retrieving gpg key timed out.

@peterbarker

This comment has been minimized.

Copy link
Contributor

peterbarker commented Aug 30, 2019

@mariansoban

This comment has been minimized.

Copy link
Contributor Author

mariansoban commented Sep 2, 2019

On Wed, 28 Aug 2019, mariansoban wrote: Is there any way how to re-run (force) travis-ci build? There was a timeout before, that caused build fail: Error: retrieving gpg key timed out.
I've hit the rerun button.

Thanks. Are there any more actions required from me?

Copy link
Contributor

peterbarker left a comment

Thanks. Are there any more actions required from me?

I mentioned the fact that an acknowledgement of whatever was taken from Ghettostation would be appropriate.

I've marked a couple of other minor things, and marked it for discussion tomorrow at the DevCall.

libraries/AP_LTM_Telem/AP_LTM_Telem.cpp Outdated Show resolved Hide resolved
libraries/AP_LTM_Telem/AP_LTM_Telem.cpp Outdated Show resolved Hide resolved
libraries/AP_LTM_Telem/AP_LTM_Telem.h Outdated Show resolved Hide resolved
Copy link
Contributor

tridge left a comment

thanks for the contribution!
I'd like to see this under a HAL_MINIMIZE_FEATURES test so use less space on 1M boards
If you had a ifdef test for build type in the LTM header, and defines LTM_TELEM_ENABLED then you could ifdef the code without having to modify sub and other firmwares that don't need this

ArduSub/GCS_Sub.cpp Show resolved Hide resolved
@tridge

This comment has been minimized.

Copy link
Contributor

tridge commented Sep 2, 2019

I noticed we're missing the WITH_SEMAPHORE() for ahrs in devo too. We should fix that

@CraigElder CraigElder removed the DevCallTopic label Sep 3, 2019
@mariansoban

This comment has been minimized.

Copy link
Contributor Author

mariansoban commented Sep 3, 2019

thanks for the contribution!
I'd like to see this under a HAL_MINIMIZE_FEATURES test so use less space on 1M boards
If you had a ifdef test for build type in the LTM header, and defines LTM_TELEM_ENABLED then you could ifdef the code without having to modify sub and other firmwares that don't need this

@tridge @peterbarker could you please check my latest changes?
Other question - should I make cleanup (rebase) PR's commits so there is exactly one commit per directory with changes?

APMrover2/Rover.h Outdated Show resolved Hide resolved
@peterbarker

This comment has been minimized.

Copy link
Contributor

peterbarker commented Sep 4, 2019

@mariansoban , for reference: #12203

@peterbarker

This comment has been minimized.

Copy link
Contributor

peterbarker commented Sep 4, 2019

@mariansoban

This comment has been minimized.

Copy link
Contributor Author

mariansoban commented Sep 4, 2019

On Wed, 4 Sep 2019, mariansoban wrote: I understand, and I'm leaving it on you Peter. I can see there one more little "issue" - you would probably need to add those dummy stuff connected to AP_LTM_Telem to vehicles that don't support LTM again (tracker, sub, tools/reply).
Yep. Sadly the fix here is to pop off the last 7 commits and put the same #ifs around this LTM does as that PR I referenced earlier does for Devo. Doesn't seem I can replicate the branch you're pulling from into my own repo - I assume it's a private repo or similar. If you can make that branch public I can make the relevant changes and push them up before merge.

Ok, I'll do it my self :)

@mariansoban mariansoban force-pushed the mariansoban:feature/ltm_protocol branch from 8d0cf85 to a177117 Sep 4, 2019
@mariansoban

This comment has been minimized.

Copy link
Contributor Author

mariansoban commented Sep 4, 2019

On Wed, 4 Sep 2019, mariansoban wrote: I understand, and I'm leaving it on you Peter. I can see there one more little "issue" - you would probably need to add those dummy stuff connected to AP_LTM_Telem to vehicles that don't support LTM again (tracker, sub, tools/reply).
Yep. Sadly the fix here is to pop off the last 7 commits and put the same #ifs around this LTM does as that PR I referenced earlier does for Devo. Doesn't seem I can replicate the branch you're pulling from into my own repo - I assume it's a private repo or similar. If you can make that branch public I can make the relevant changes and push them up before merge.

Ok, I'll do it my self :)

DONE

Copy link
Contributor

peterbarker left a comment

LGTM

@peterbarker

This comment has been minimized.

Copy link
Contributor

peterbarker commented Sep 4, 2019

... I guess we could've added WITH_SEMAPHORE on the other uses of AHRS too, however. And minimised the scope of its use.

But I think this could go in as-is and we can fiddle around the edges.

@peterbarker

This comment has been minimized.

Copy link
Contributor

peterbarker commented Sep 4, 2019

@mariansoban Sorry, no merge commits allowed in ArduPilot :-)

You'll want to pop that merge commit off and then do a rebase onto master. Let me know if you want any help doing this.

@mariansoban mariansoban force-pushed the mariansoban:feature/ltm_protocol branch from 3523222 to 3ac6a3f Sep 5, 2019
@mariansoban

This comment has been minimized.

Copy link
Contributor Author

mariansoban commented Sep 5, 2019

@mariansoban Sorry, no merge commits allowed in ArduPilot :-)

You'll want to pop that merge commit off and then do a rebase onto master. Let me know if you want any help doing this.

I was afraid of this a bit. It's fixed now and I also made git log history cleanup.
Next thing I'm afraid is this PR from trigde #12152 :)

@mariansoban mariansoban force-pushed the mariansoban:feature/ltm_protocol branch from 3ac6a3f to 5015584 Sep 6, 2019
@mariansoban

This comment has been minimized.

Copy link
Contributor Author

mariansoban commented Sep 6, 2019

Serial protocol index for LTM changed to 24 (23 is already used by RCIN)

@mariansoban mariansoban force-pushed the mariansoban:feature/ltm_protocol branch from 5015584 to d14483e Sep 6, 2019
@mariansoban

This comment has been minimized.

Copy link
Contributor Author

mariansoban commented Nov 13, 2019

@peterbarker feel free to force-push that branch

@peterbarker peterbarker force-pushed the mariansoban:feature/ltm_protocol branch from 537e33f to 28554a0 Nov 14, 2019
@peterbarker

This comment has been minimized.

Copy link
Contributor

peterbarker commented Nov 14, 2019

@mariansoban I've force-pushed the branch for this PR.

@mariansoban

This comment has been minimized.

Copy link
Contributor Author

mariansoban commented Nov 14, 2019

Thank you, now is @tridge 's turn, isn't it?

@peterbarker

This comment has been minimized.

Copy link
Contributor

peterbarker commented Nov 19, 2019

@mariansoban tridge had to run off during the call, sorry...

@davidbuzz

This comment has been minimized.

Copy link
Contributor

davidbuzz commented Nov 19, 2019

I'm just doing a light review and catch-up, and i'm trying to come to terms with what ground-station/s support the LTM protocol.... it seems right now that 'ghettostation' antenna tracker works, but my question is does MissionPlanner or MAVProxy support this protocol?

@peterbarker peterbarker force-pushed the mariansoban:feature/ltm_protocol branch from 28554a0 to f6475e7 Nov 19, 2019
@peterbarker peterbarker force-pushed the mariansoban:feature/ltm_protocol branch from f6475e7 to b1881e3 Nov 19, 2019
@mariansoban

This comment has been minimized.

Copy link
Contributor Author

mariansoban commented Nov 19, 2019

I'm just doing a light review and catch-up, and i'm trying to come to terms with what ground-station/s support the LTM protocol.... it seems right now that 'ghettostation' antenna tracker works, but my question is does MissionPlanner or MAVProxy support this protocol?

Currently there is no support for LTM in MissionPlanner and MAVProxy. Short list of supported LTM "consumers" can be found on iNAV documentation for LTM: https://github.com/iNavFlight/inav/wiki/Lightweight-Telemetry-(LTM).
Anyways, I find the main benefit of LTM in sending of most important aircraft data at slow baud rates. For example you can skip using of telemetry modules and use FPV system's audio channel with FSK modems for one-way (downlink from aircraft) data transfer instead. You can use those data for antenna tracking, video display data overlay or convert them to Mavlink and send to MissionPlanner on ground side

@tridge
tridge approved these changes Nov 26, 2019
@tridge
tridge approved these changes Nov 26, 2019
@tridge tridge merged commit 25cf389 into ArduPilot:master Nov 26, 2019
4 checks passed
4 checks passed
ArduPilot.ardupilot #20191119.6 succeeded
Details
ArduPilot.ardupilot (Cygwin SITL build) Cygwin SITL build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
semaphoreci The build passed on Semaphore.
Details
@tridge

This comment has been minimized.

Copy link
Contributor

tridge commented Nov 26, 2019

nice work, thanks!
@mariansoban can you work with @Hwurzburg to get this in the wiki?

@mariansoban

This comment has been minimized.

Copy link
Contributor Author

mariansoban commented Nov 26, 2019

nice work, thanks!
@mariansoban can you work with @Hwurzburg to get this in the wiki?

Sure, I can try. What's the common way for collaboration on wiki documentation?
My email is marian.soban@gmail.com

@Hwurzburg

This comment has been minimized.

Copy link
Contributor

Hwurzburg commented Nov 26, 2019

@mariansoban Marian, there are several ways:

  1. Create the wiki pages using an existing page as a guide and http://ardupilot.org/dev/docs/common-wiki_editing_guide.html and submit a PR on the repo, then I can help revise...this is the most helpful and quickest way -or-
  2. Provide bullet points on "what,when,why, and how" and I create the documents....then there is a lot of back and forth enabling me to understand the tech details, and I create the documents....usually this is the slowest way .....however, in this case, I am very familiar with the subject and can create it with minimal interaction

Whichever way suits you...#1 is always preferred because it allows the documentation to be created by the most knowledgeable and then edited...
In case of #1 this page http://ardupilot.org/plane/docs/common-frsky-telemetry.html could be a template...LTM would be much shorter and you have a lot of the information/images already in this PR..protocol details can be by reference once what data is being sent is talked about....but what is missing is how to get the telem from the serial port to the ground station/tracker (usually via vtx audio, correct?)...I think that needs to shown, if possible, like at the beginning of the FRSky page....it can always be sent via a telem radio, but then a lot of it benefit of a compact protocol is lost, correct?
Let me know which way you would like to proceed....

@mariansoban

This comment has been minimized.

Copy link
Contributor Author

mariansoban commented Nov 27, 2019

Ok, I'm choosing way #1 and I'll take a look at it probably this weekend. Thanks for pointing me.

@vierfuffzig

This comment has been minimized.

Copy link
Contributor

vierfuffzig commented Dec 12, 2019

@mariansoban

This comment has been minimized.

Copy link
Contributor Author

mariansoban commented Jan 6, 2020

@Hwurzburg Hi Henry, could you please check this PR to ardupilot wiki?
ArduPilot/ardupilot_wiki#2409
I gave you access to repository https://github.com/mariansoban/ardupilot_wiki/tree/feature/ltm-telemetry-doc to make you possible to fix issues.
Thank you

@Hwurzburg

This comment has been minimized.

Copy link
Contributor

Hwurzburg commented Jan 6, 2020

thanks will check it out in a day or two...thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
9 participants
You can’t perform that action at this time.