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

Add moretext char[150] extension to statustext message #12937

Open
wants to merge 3 commits into
base: master
from

Conversation

@peterbarker
Copy link
Contributor

peterbarker commented Dec 1, 2019

Came out of a discussion with @tridge and @auturgy today.

@peterbarker

This comment has been minimized.

Copy link
Contributor Author

peterbarker commented Dec 1, 2019

MAVProxy commit is here: ArduPilot/MAVProxy#708

@peterbarker

This comment has been minimized.

Copy link
Contributor Author

peterbarker commented Dec 1, 2019

Sample output from the included debug commit:

APM: 69
APM: 012345678901234567890123456789012345678901234567890123456789012345678
APM: 70
APM: 0123456789012345678901234567890123456789012345678901234567890123456789
APM: 71
APM: 01234567890123456789012345678901234567890123456789012345678901234567890
APM: 72
APM: 012345678901234567890123456789012345678901234567890123456789012345678901
APM: 73
APM: 0123456789012345678901234567890123456789012345678901234567890123456789012
APM: 74
APM: 01234567890123456789012345678901234567890123456789012345678901234567890123
APM: 75
APM: 012345678901234567890123456789012345678901234567890123456789012345678901234
APM: 76
APM: 0123456789012345678901234567890123456789012345678901234567890123456789012345
APM: 77
APM: 01234567890123456789012345678901234567890123456789012345678901234567890123456
APM: 78
APM: 012345678901234567890123456789012345678901234567890123456789012345678901234567
APM: 79
APM: 0123456789012345678901234567890123456789012345678901234567890123456789012345678
APM: 80
APM: 01234567890123456789012345678901234567890123456789012345678901234567890123456789
APM: 81
APM: 012345678901234567890123456789012345678901234567890123456789012345678901234567890
APM: 82
APM: 0123456789012345678901234567890123456789012345678901234567890123456789012345678901
APM: 83
APM: 01234567890123456789012345678901234567890123456789012345678901234567890123456789012
@peterbarker

This comment has been minimized.

Copy link
Contributor Author

peterbarker commented Dec 1, 2019

So there are several cons to this commit:

  • that's a lot of extra RAM floating around in the statustext queue.
  • that's a lot of extra stack space in vfprintf and send_textv.
  • probably still not long enough for @WickedShell 's dump-stack-trace-from-lua requirements.

Advantages with this approach vs STATUSTEXT_LONG are that it's somewhat more backwards-compatible with mavlink1-only devices like OSDs; they may only display truncated text, but they'll get something.

I'm still not sure I don't prefer my

      <field type="uint8_t" name="num_chunks">Number of statustext chunks in this message.</field>
      <field type="uint8_t" name="chunk_num">This chunk's offset, starting at zero.</field>

statustext extension concept better, but tridge suggests it suffers from the "need-to-wait-for-chunks before showing anything" problem, and those chunks may never come. You'd need a timer running to timeout waiting for the last chunk before display....

@WickedShell

This comment has been minimized.

Copy link
Contributor

WickedShell commented Dec 2, 2019

Yeah I'm torn. I want longer status texts for getting compiler errors down, but on the one hand this eats a lot of RAM (versus the current plan of just send up to like 5 messages in a row).

The other concern I have is, given the deployment time on GCS's coping with this, and having to support MAVLink 1 means we would still have to have the critical parts of the message in the first 50 bytes. The extension one to spam more STATUSTEXT messages means that MAVLink 1 GCS's loose no data if they don't do single reassembly. And either way MAVLink2 GCS's will have to be modified to take advantage of the extension in either scenario. (the chunk stuff also actually extends to much longer messages easily whereas increasing the blob size doesn't help us there)

@CraigElder CraigElder removed the DevCallTopic label Dec 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.