[gpsd-dev] AIS: IMO236/289 Met/hydro message

[christian Gagneraud]

Hi all,

I'm currently working on getting gpsd parse correctly these 2 types of 
messages. I've spotted some errors in the data structure and in the 
decoding that are easy to fix. But as I'm moving forward, I'm facing a 
dilemma.
The dac1fid31 data structure is shared between IMO236 (FID=11) and 
IMO289 (FID=31), which at first makes sense as they are very similar, 
but as you start looking at the lowest level details, there's some 
differences that make awkward if not impossible to manage these 2 
messages with a common structure.

So first, let's review the differences:
1) Only in IMO289:
 - Position accuracy bit
 - Greater visibility bit
=> No big deal, the IMO236 always set them to false

2) Long/Lat:
 - IMO236: latitude comes first, followed by longitude
 - IMO289: longitude comes first, followed by latitude
=> No problems, managed in driver_ais.c, where code is not shared

3) Signed vs unsigned:
 - IMO236: signedness is achieved by subtracting offset on unsigned values
 - IMO289: uses 2's complement for signed value (except water level)
=> Ditto.

4) Data not available:
 - IMO236: all the bits are sets.
 - IMO289: special values are used (usually incompatible with IMO236).
=> That's where the problems start, see below

5) Units/Precision:
 - Water level: 0.1m on 9bits for IMO236, 0.01m on 12 bits for IMO289
=> Managed in driver_ais.c, not a real concern

And now, lets look at which code is currently shared or not:

1) driver_ais.c
Not shared due to message structure incompatibility, IMO236 needs a 
couple of minor fixes, I'm suspecting the IMO289 to require more work.

2) gps_json.c
Code is shared, the non scaled code seems OK as it passes the raw data, 
but the scaled code is wrong because it doesn't deal with the special 
"data not available" values and would happily scale anything, resulting 
in crazy values.

3) ais_json.c
Code is shared, but it uses the IMO289 definitions for "data not 
available", which definitely breaks the IMO236 message.

4) gps.h
The "data not available" codes for IMO236 are missing

So finally, of all these differences, the biggest problem is the "data 
not available" stuff. To fix this problem, I see 2 different approaches:

1) Make the IMO236 low-level code in driver_ais.c mimic the IMO289 one, 
so that the upper layers code can easily be shared.
for example, in driver_ais.c, for IMO236:
----
ais->type8.dac1fid31.airtemp    = UBITS(153, 11)
    - DAC1FID31_AIRTEMP_OFFSET;
/* IMO236: 1447 => IMO289: -1024 */
if (ais->type8.dac1fid31.airtemp == DAC1FID11_AIRTEMP_NOT_AVAILABLE)
   ais->type8.dac1fid31.airtemp = DAC1FID31_AIRTEMP_NOT_AVAILABLE;
----

* pros: Easy to do, allow to maximize the shared code in the upper layer
* cons: The IMO236 structure is actually an IMO236/IMO289 bastard, in 
that it doesn't fully follow the IMO236 specs.

2) Define a new dac1fid11 structure, and have different code at all levels

* pros: Both IMO236 and IMO289 stick to the specs
* cons: More code to write/read/maintain, with some similarities but 
subtle differences.

So the question of the day is: which way should be taken to address this 
issue? I personally think that the #2 is cleaner, but i would have no 
problem taking the path #1

FYI, attached is a diff that illustrates my point (with some minor fixes 
to the IMO236 decoding). As long as I deal with available data and look 
only at raw values, I managed to validate the decoding of live IMO236 
met/hydro message against known-good decodings.

Regards,
Chris

ais-imo.diff
Description: Text Data