Restructure of APRSPacket

[ab0oo]

I'm unhappy with the data structures we're using for the base APRSPacket. Hear me out:

I've been working on adding weather data parsers. It's not too complex, but it highlights the different ways the data can be presented. If you refer to chapter 12 of the APRS Spec, there are at least 5 different ways to encode weather information. Some contain positions, some contain timestamps. This makes the object hierarchy get messy pretty quickly.
Looking at the chart in Chapter 5 (page19) and the "APRS Data and Data Extension" table on page 18, it looks like there should be an interface called "APRSData" and an interface called "APRSDataExtension". An APRSPacket can contain multiple instances of the interface (i.e. a packet can contain Lat/Long Coordinates AND Raw Weather Station Data). I'm proposing that the APRSPacket contain a Set of APRSData objects, and a Set of APRSDataExtension objects. This is probably going to break some stuff downstream, so I'm looking for input here.
I'm also thinking about how to indicate at a high-level what information a packet actually contains. I'm not sure we should have to walk an entire set to see if a packet contains a position. Perhaps a set of boolean values like hasPosition(), hasWeather(), hasTelemetry(), etc?

[ge0rg]

I've been pondering about this issue for a long while already, and I fully agree that we need a more flexible data structure.

So far we have:

• InformationField as the generic base type for "APRS Data", of which there can only be one in a packet, at least currently
  • ObjectPacket for objects
    • ItemPacket as a subclass for items
  • MessagePacket for messages and bulletins etc.
  • PositionPacket for station position reports
  • UnsupportedInfoField for everything that we can't parse.

We also have different extension elements already, of which currently only one can be inside the above InformationField:

• DataExtension - generic base type
  • CourseAndSpeedExtension
  • PHGExtension
  • RangeExtension

I'm not sure if we should rename InformationField into APRSData, but I rather dislike all the *Packet field names because they are actually just Fields, so if we are going to do a major breaking change refactoring, let's axe that name.

I agree that having only one extension in a packet is not sufficient, but I'm not sure if it makes sense to have multiple information fields in the frame, or if we can use the extensions for complex data types. Also I think it is good to have a primary information field that defines what kind of packet this is - if it's a weather packet, maybe it doesn't make sense to treat it as something different? Also I'm not sure if it's enough to have one extension per extension type, or if we need to support lists. Maybe @hessu can provide insight on a good data structure to cover APRS packets?

My suggestion for a large-scale refactoring (for which I'm lacking the time) would be as follows, each item can be disputed / discussed individually:

1. Move classes into according packages (directories) based on their function, i.e. get most things out of the parser package, have an aprsdata or field package, an extension package, and some package for individual tokens like Callsign and Digipeater (maybe "token"?)
2. Rename *Packet into *Field
3. Change InformationField's extension field into a HashMap<Class<DataExtension>, DataExtension> for one extension per type, or with a List<DataExtension> value type if we want to support multiple elements
4. Convert Position, which is a custom element only contained in PositionPacket into a DataExtension so that it can be combined with everything else
5. Add to InformationField a generic method <T extends DataExtension> getExtension(Class<T>) so that we can directly run code like this:
   CourseAndSpeedExtension cse = packet.getExtension(CourseAndSpeedExtension);
6. Add a generic convenience method boolean hasExtension(Class<T>)

My Java generics knowledge is a bit rusty, so don't pin me on the exact syntax, so please consider the above as pseudo-code.

[ab0oo]

I just pushed a v2.0 branch that makes many of the changes you suggested, @ge0rg . I didn't do a whole lot with the POSITION stuff, but I basically created a hierarchy inside an APRSPacket that contains a Set of APRSData fields (i.e. position, timestamp, weather, object and item), a single APRS Data Extension, and a field for a comment.
I didn't restructure the package per bullet 1, but I'm looking at that next. I took care of most of bullet 2. Bullet 3 is close. I used a Set instead of a List, and I need to implement the HashMap for DataExtension.
Bullet 4 is done.
Bullets 5 and 6... working on those. Definitely a step in the right direction. Also, there are a LOT of malformed packets floating around out there... :)
I'm going to keep this issue open until I get all of your bullet points done, and maybe some feedback.

[arodland]

@ab0oo well, it looks like you've already done what I offered to do, and then some :) one thing I can offer: there are some test cases from the original Ham::APRS::FAP in https://metacpan.org/release/HESSU/Ham-APRS-FAP-1.21/source/t/30decode-wx-basic.t and https://metacpan.org/release/HESSU/Ham-APRS-FAP-1.21/source/t/31decode-wx-ultw.t

[penguin359]

I have been getting back into Android development after a year hiatus and have been meaning to finish up my own attempt at supporting weather packets. I remember getting stalled out last time for exactly the reason about how to handle the many different weather formats specified in the APRS 1.01 specification. Some are natural extensions of the GPS location beacons, but other formats are clearly a completely different type, of course, but I don't want to re-iterate what's already been said.

It looks like quite a bit of work has been done since I was last here so I'll need a day or so to catch up. In the mean time, I've been working on updating the unit test frameworks so they can be used while working on these weather station packets. Now that GitHub can do it's own CI, I've added that as a pull request for both the master branch (#16) and this v2.0 branch (#17). In doing so, I noticed some commits did not make it onto the v2.0 branch including one commit that's needed to fix Apache Ant builds. Ant is currently broken on v2.0. I've included that commit cherry-picked into my pull request so that the CI tests will pass the build for both Maven and Ant.

Several other commits are missing related to the BEACON issue (#10) and my additional unit-tests (#13). I can work on pulling them in later.

[penguin359]

While reviewing the v2.0 branch, my first question is, why bump the target version all the way to Java 15?

I did feel that supporting Java 6 was a bit tiresome, but 15 seems to limit the environments this can be used in. The Long-Term Support releases of Java are 8, 11, and now 17 which was just released in 2021. That means that only the latest Java is an option for those wanting to use javAPRSlib v2.0 with an LTS release.

Outside of Android development, I am not a big Java developer so I am not as certain what the ecosystem looks like, and I'm not exactly certain how Java versions relate to specific Android versions. From what I can tell, the recommended version of Java is Java 11 LTS, though, the default target/source for new version is 1.8 with the default build files that are generated.

[ab0oo]

While reviewing the v2.0 branch, my first question is, why bump the target version all the way to Java 15?

I did feel that supporting Java 6 was a bit tiresome, but 15 seems to limit the environments this can be used in. The Long-Term Support releases of Java are 8, 11, and now 17 which was just released in 2021. That means that only the latest Java is an option for those wanting to use javAPRSlib v2.0 with an LTS release.

Outside of Android development, I am not a big Java developer so I am not as certain what the ecosystem looks like, and I'm not exactly certain how Java versions relate to specific Android versions. From what I can tell, the recommended version of Java is Java 11 LTS, though, the default target/source for new version is 1.8 with the default build files that are generated.

In the interest of keeping this conversation moving :) ...
I think we're to the point where we can use Java 17. The pace of Java releases has increased of late, and to be honest, we're not really using anything "new". There are a couple of annotations in there, and some generics, but that's Java 8. All of the build tools are set up for Java 17, and seem to be completing just fine. AWS, GCP, etc all support Java 17 as a standard target, and we'd have to go back a couple of years of Ubuntu to find one that Java 17 isn't supported in.
If someone really has issues, it looks like it still compiles and tests correctly at Java 8, you just need to make the changes in the pom.xml.

[penguin359]

If I recall correctly, the issue was with the Scala plugin for Android that we are using. It's really quite a bit of a pain point that is difficult to overcome. The specific Gradle build plugin we are using for compiling Scala for Android is limited to the compiler for Scala 2.11.x and that plugin is no longer being developed. This version requires targeting Java 11 at the newest or it will complain. Migrating to Scala 2.12.x or newer will require completely replacing the build plugin at this point. This just means we'd need to maintain a branch that still targets Java 11 binary compatibility.

I might have the exact details wrong as it's been a couple of years, but it was some combination of this. I'd love to update the Scala plugin, but it always ends up being very complicated every time I try to dig into this.