Fix and reinstate OpenTrac decode routines #188
Comments
|
Personally.. I would love to see OpenTrack get real adoption as APRS is rather convoluted. To get adoption, support has to start with the software solutions. |
|
That was the intent of Xastir having decode capability for it. There was a big push to get it adopted, and tremendous resistance in the community back then. That code in Xastir rotted and was so full of dangerous string manipulation that we had to remove it during a big "clean up warnings" run. I, too, would like to see it come back. If you would be interested in helping to resurrect the old decode routines and fix the errors in them, I'd be delighted. Otherwise this may sit unaddressed for quite a while until I'm available to work on it. |
|
I originally wrote that code, and used to exercise it with the
dual-protocol Opentracker firmware I used to run in my mobile. I'd love to
see it get resurrected, if there's a use for it again.
I may be able to help getting the code back in shape too.
…On Mon, Dec 13, 2021 at 12:51 PM Tom Russo ***@***.***> wrote:
That was the intent of Xastir having decode capability for it. There was a
big push to get it adopted, and tremendous resistance in the community back
then.
That code in Xastir rotted and was so full of dangerous string
manipulation that we had to remove it during a big "clean up warnings" run.
I, too, would like to see it come back. If you would be interested in
helping to resurrect the old decode routines and fix the errors in them,
I'd be delighted. Otherwise this may sit unaddressed for quite a while
until I'm available to work on it.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#188 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEMNKXYBLAIJZXQX6JBPFDTUQZMGPANCNFSM5J6WY6TQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
--
Curt, WE7U http://xastir.org http://www.sarguydigital.com
|
|
The code lives on in git history, and was removed in a single commit, 2ebfb27. Unfortunately, attempting to revert that commit gives conflicts that will not be simple to resolve thanks to many code reformats. However, if one does a "git log -p 512b5f2..2ebfb27" one can see what the commit did. One could presumably also capture that into a file and play with "patch" options to force it to undo. In fact, when I did that and patched with "patch -R -l" (ignore whitespace, revert patch) only one chunk failed, and it should be fairly easy (since you wrote the code) to see how to make that one work again. It looks like it's simply failing to find the context because what used to be an "else if" testing the APRS PID is just an "if" now. |
|
I'm trying to find the details here but I believe ArgentData removed OpenTrac protocol support from their OpenTracker products due to a lack of interest. I think it was announced on their Yahoo group at http://groups.yahoo.com/group/opentracker/ but it's long gone. Here are a few links I've found so far: https://forum.arduino.cc/t/opentrack-dead/23399 No mention of it's revocation at http://opentrac.org/ I also don't see any new firmware for the OpeTracker / Tracker line since 2008 - http://n1vg.net/opentracker/downloads.php |
|
Yeah, that was sort of why we removed support from Xastir rather than fixing all the unsafe string handling in those routines\ that were throwing the warnings. As I pointed out in the initial comment, my entire interest in seeing the feature reinstated is that some other APRS clients are starting to support creation and transmission of OpenTrac packets (YAAC, I think), and some folks in my local area are using that feature. So their packets are being flagged by Xastir as invalid KISS packets and discarded. Since YAAC is an entirely java-based system, whoever takes this issue should at least be able to test the decode routines by creating packets with YAAC. |
|
I created a patch file to reverse the earlier commit (thanks Tom for making
that easy) and installed gcc-10 here plus figured out how to invoke it
instead of gcc-7. I also have gcc-11 available to play with.
I fixed the first warning in src/interface.c in a simple/reasonable way.
As for the next two warnings, here's a sample of one "fix":
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wstringop-truncation"
strncat(buffer,entity_name,9); // Entity name
#pragma GCC diagnostic pop
That gets rid of the warning but probably isn't a good long-term fix. From
what I've read it should work for Clang and GCC though.
Most likely the -correct- fix would be to switch from "strncat" to "memcpy"
functions.
Comments?
…On Mon, Dec 13, 2021 at 3:26 PM Tom Russo ***@***.***> wrote:
Yeah, that was sort of why we removed support from Xastir rather than
fixing all the unsafe string handling in those routines\ that were throwing
the warnings.
As I pointed out in the initial comment, my entire interest in seeing the
feature reinstated is that some other APRS clients are starting to support
creation and transmission of OpenTrac packets (YAAC, I think), and some
folks in my local area are using that feature. So their packets are being
flagged by Xastir as invalid KISS packets and discarded.
Since YAAC is an entirely java-based system, whoever takes this issue
should at least be able to test the decode routines by creating packets
with YAAC.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#188 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEMNKX7ZP5BBBER7Z24B7V3UQZ6JVANCNFSM5J6WY6TQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
--
Curt, WE7U http://xastir.org http://www.sarguydigital.com
|
|
Those pragmas simply tell the compiler to ignore the problem and not issue warnings. There was a lot if incorrect use of strncat in Xastir, mostly using incorrect values of "n" --- it's supposed to be the maximum number of characters that can fit into the destination string, and in many cases the code had been written to specify that number as the number of characters to copy out of the source string instead. We fixed a lot of that, but the opentrac code had a lot of it. The "right" fix is to fix the strncat calls so that one keeps track of how much room is left in the destination string, and use that as the"n" in strncat. |
In #56, we removed all the code that decodes OpenTrac packets, because they were throwing warnings from gcc 8 that looked like they might be real errors, and we thought nobody was using OpenTrac anymore --- so removing the feature was easier than fixing it, as we were in the middle of a big effort to remove warnings from gcc 8.
Now, it seems, YAAC and some other APRS clients are supporting transmission of OpenTrac packets, and folks local to me are actually using that feature.
So I should go back to commit 2ebfb27 and undo it, then fix all the warnings that gcc10 spews as a result. It is possible that a git revert of that commit would just reinstate opentrac and all its warnings/errors, but it is also possible that development and code reformatting done since then will make for many revert conflicts.
The text was updated successfully, but these errors were encountered: