ttlappalainen / NMEA2000

NMEA2000 library for Arduino
525 stars 218 forks source link

PGN129794 AISInfo parsed incorrectly #267

Open lsoltero opened 2 years ago

lsoltero commented 2 years ago

Looking at canboat and comparing their completed entry for PGN129794 I believe there is an error in setting and parsing of (tN2kAISTranceiverInfo)AISinfo.

The code has N2kMsg.AddByte(AISinfo & 0x1f); I believe it should be N2kMsg.AddByte((AISinfo & 0x1f)<<3);

and

vb=N2kMsg.GetByte(Index); AISinfo=(tN2kAISTranceiverInfo)(vb & 0x1f); should be vb=N2kMsg.GetByte(Index); AISinfo=(tN2kAISTranceiverInfo)((vb>>3) & 0x1f);

here is the json def for this PGN in { "Order":20, "Id":"aisTransceiverInformation", "Name":"AIS Transceiver information", "BitLength":5, "BitOffset":592, "BitStart":0, "Type":"Lookup table", "Signed":false, "EnumValues":[ {"name":"Channel A VDL reception","value":"0"}, {"name":"Channel B VDL reception","value":"1"}, {"name":"Channel A VDL transmission","value":"2"}, {"name":"Channel B VDL transmission","value":"3"}, {"name":"Own information not broadcast","value":"4"}, {"name":"Reserved","value":"5"}]}]},

note the BitStart:0 and bit offset. As written the offset in the library is 595 which is inconsistent with canboat. Basically canboat pads at the end and this library pads at the start.

Please let me know if I am misreading this.

Thanks,

--Luis

ttlappalainen commented 2 years ago

You are misreading it. Bit 0 is on right, not left. You are thinking it wrong since data comes it little endian, but normally you read values in big endian. So e.g., 16 bit number 9270 is 0x2436. In incoming little endian bytes this is shown as 0x36 0x24. But also bits comes in little endian 0110 1100 0010 0100

But message setter has small error. It should be N2kMsg.AddByte( 0xe0 | (AISinfo & 0x1f) ); N2kMsg.AddByte( 0xff );

The last parameter SID could be optional parameter.

lsoltero commented 2 years ago

Hi Temo,

Thanks for that... I need to augment the library to add AISinfo to 129038, 129039, 129809, 129810 so that our NMEA2000 -> NMEA0183 converter can generate AIS VDO messages for the vessels own MMSI. So basically generate VDO if AISinfo == N2kaisti_Own_information_not_broadcast and otherwise generate VDM messages.

129038 and 129039 seem to be particularly confusing since AISinfo comes after 19 bits of communication state. So.. here is what I have for Set N2kMsg.AddByte(0xff); // Communication State (19 bits)
N2kMsg.AddByte(0xff); N2kMsg.AddByte(((AISinfo & 0x1f)<<3) | 0x07); // AIS transceiver information (5 bits)

and for read vb=N2kMsg.GetByte(Index); // Communication State (19 bits)
vb=N2kMsg.GetByte(Index); vb=N2kMsg.GetByte(Index); // AIS transceiver information (5 bits)
AISinfo = (tN2kAISTranceiverInfo) ((vb>>3) & 0x1f);

I have attached a patch file incase anyone else needs this. The patch file could be augmented so that it does not break the prototypes for the current library.

Thanks,

--Luis

0005-Add-AISinfo.txt

ttlappalainen commented 2 years ago

Why it is confusing?

You could also use: N2kMsg.Add3ByteInt( ( (AISinfo & 0x1f)<<19 ) | 0x07ffff );

and

uint32_t StateAndInfo=N2kMsg.Get3ByteUInt(Index); AISinfo = (tN2kAISTranceiverInfo) ( (StateAndInfo>>19) & 0x1f );

Instead of rewriting functions you can just extend them and handle compatibility by writing inline functions and using default parameter.