aprsdroid icon indicating copy to clipboard operation
aprsdroid copied to clipboard

bug: Improper data/comment field construction

Open Tyler-2 opened this issue 2 years ago • 1 comments

At this line, a space is inserted between the status var (Referred to as Comment field in the UI, I think), and the preceding speed, freq, alt vars.

https://github.com/ge0rg/aprsdroid/blob/2041ba3497647c279b1bd61a37f98042c1329cc7/src/AprsService.scala#L242

The comment in APRS terminology is always the last field: Screenshot_2022-05-27_14-52-15 (APRS 1.0 doc, page 94, APRS Data Formats)

Going forward I will refer to the comment meaning the user-provided comment, consistent with the APRS spec documentation of comment. APRSDroid internally refers to (I think) the comment as status. And erroneously (again, I think) calls the variable which contains APRS data extensions and the APRS comment, "comment". I believe this is a mistake.

The frequency and altitude data is not data extension but are actually just text that is treated as part of the comment field.

APRSDroid uses un-timestamped position reports.

Course/speed is an optional data extension component in the APRSDroid packets.

Lat/Long reports without timestamps or data extension components

This is a Lat/Long position report without timestamp, sent from APRSDroid: =3742.94N\07749.29Wk Some comment Screenshot_2022-05-27_14-59-14

Note the space between the Wk and Some comment. W is the end of the Long field. k is my symbol code. The comment then would be Some comment. Wrong.

  • In compressed position reports, there is mention that if the character following the symbol code is a space, it indicates there is no Course/Speed/Range/Altitude value. But APRSDroid does not send compressed position reports.
  • Mic-E status text fields make mention of a space before comments under some circumstances, but APRSDroid does not send Mic-E format data.
  • Status reports (a message type that starts with a > instead of the = in our example packet specify that if the status report includes a Maidenhead Grid Locator, the status text must start with a space. But we are not APRS does not sent status report packets, and the data format APRSDroid uses does not have status text.

Screenshot_2022-05-27_15-23-23 Page 32, "Position and DF Report Data Formats". Unfortunately their examples in the above are a little unclear. In their first example Test 001234 is the comment. In the second example, the comment is Test /A=001234.

Screenshot_2022-05-27_15-28-09

The altitude is ALWAYS 6 characters long in the comment. Therefore it does not need to be followed by a space, but there might be other reasons to do that.... If you see /A= in the comment field, the next 6 characters are the altitude in feet.

So it seems to me that the space is inappropriate in the case of =3742.94N\07749.29Wk Some comment which should instead read =3742.94N\07749.29WkSome comment

and packets that contain altitude data maybe shouldn't look like: =3742.94N\07749.29Wk/A=000169 Some comment but should instead look like =3742.94N\07749.29Wk/A=000169Some comment or =3742.94N\07749.29WkSome comment /A=000169 if the example is to be followed without explanation.

Section conclusion: The space preceding the APRSDroid comment is not appropriate except maybe when the /A=xxxxxx precedes it.

Lat/Long reports without timestamps, with data extension components

APRS data extension fields are always 7 byte. Which means they do not need a delimiter (like a space) to terminate them. But they do occasionally need a space sometimes to pad them out. But that's not what's happening in APRSDroid.

Confusion when using the frequency object

http://www.aprs.org/info/freqspec.txt APRS FREQUENCY FORMATS section adds some confusion here. The frequency object is also part of the comment field. Some effort was made to make sure this was easily human readable but also parse-able.

A simplified summary of the rules for the frequency object in APRSDroid is:

  • Frequency object should be the first 10 bytes. Either FFF.FF MHz or FFF.FFFMHz for instance. It must always be 10 bytes long.
  • MHz must be mixed case on transmission.
  • It is permissable to allow wrong case (mhz or MHZ etc) in decode/receipt.

They include this example: FFF.FFFMHz comment...

This is the only reference to a trailing space for the frequency object, and the space is never mentioned otherwise as far as I can tell.

However there is reference to an optional "not in the spec" leading space:

OTHER COMBINATIONS: It is important to note that the TEXT field used for this frequency already has several possible options. Normally the comment-TEXT field begins after the SYMBOL byte. But there are already several defined 7 byte Data-Extensions that then push the comment-TEXT field further to the right as shown here. In many applications, an optional delimiter is added to make the packet more readable. This optional delimiter (usually a " " or "/") is not explicitly called out in the spec, but should be considered in all following parsing of the text field. All of these can be valid use of Data-Extensions and following FREQUENCY info.

!DDMM.mmN/DDDMM.MMW$FFF.FFFMHz ... begins after symbol ($) !DDMM.mmN/DDDMM.MMW$CSE/SPD/FFF.FFFMHz ... !DDMM.mmN/DDDMM.MMW$PHGphgd/FFF.FFFMHz ... !DDMM.mmN/DDDMM.MMW$DFSshgd/FFF.FFFMHz ... !DDMM.mmN/DDDMM.MMW$DFSshgd FFF.FFFMHz ... with SPACE insted of / !DDMM.mmN/DDDMM.MMW$CSE/SPDFFF.FFFMHz ... w/o delimiter !DDMM.mmN/DDDMM.MMW$PHGphgdFFF.FFFMHz ... w/o delimiter

LEADING SPACE ISSUE: Althout the optional delimiter shown here is a "/" it can also be a SPACE character too. The original intent of APRS was that any free text field that begins with either of these two delimiters, then they can be ignored and the beginning of the text field begins after them. We believe that Kenwood and maybe others did not allow for this option, and so a leading space will not work. We are trying to clarify this.... now in 2012...

It isn't clear to me what they meant to do with the leading delimiter on the frequency data. The leading delimiter isn't necessary for decoding...

The reference to the intent there implies that if you use the free form text comment field to contain interesting custom attributes like frequency, you can delimit those with a space or /. That way, maybe, APRS clients that don't understand them are able to skip over them. But I don't understand how you're supposed to know that the field is "over". Spaces can't be delimiters all the way through the comment, in this way....

So there can be a space or / or other delimiter before the frequency object "to make it more readable" (for who?). That's a leading space.

A packet with a frequency object and altitude in APRSDroid currently looks like this: =3742.94N\07749.29Wk146.520MHz/A=000158 Some comment and I believe it should look like this, if the "leading space" is actually desirable: =3742.94N\07749.29Wk146.520MHz /A=000158Some comment

Impact

  • Comments from APRSDroid always start with a space.
  • Packets are ever so slightly longer, impacting RF health.
  • Making use of the APRSDroid comment field, with data field options and frequency disable, to insert custom data extensions like PHG data doesn't work.
  • This very popular implementation is non-compliant!

Suggested fixes

Make the comment field consist of one of, depending on which fields are defined:

status_spd +...

  • status_freq + " " + status + " " + status_alt
  • status + " " + status_alt
  • status_freq + " " + status
  • status

I wouldn't actually even want to put a leading or following space in front of status_alt but based on the example packets it seems allowable.

TLDR

It appears APRSDroid is unconditionally inserting a space between the comment field and the symbol code, when there should ~not be a space there at all~ only maybe be spaces involved between the altitude, frequency, and user provided comment fields.

Tyler-2 avatar May 27 '22 20:05 Tyler-2

There may be all sorts of errors there since I wrote it while perusing the spec.... The suggested fix and TLDR are probably sufficient to get the gist.

Tyler-2 avatar May 27 '22 20:05 Tyler-2