Feature/ais6 235 10 #195
Feature/ais6 235 10 #195
Conversation
src/libais/ais6.cpp
Outdated
| @@ -27,7 +27,7 @@ Ais6::Ais6(const char *nmea_payload, const size_t pad) | |||
| bits.SeekTo(38); | |||
| seq = bits.ToUnsignedInt(38, 2); | |||
| mmsi_dest = bits.ToUnsignedInt(40, 30); | |||
| retransmit = !bits[70]; | |||
| retransmit = static_cast<bool>(bits[70]); | |||
There was a problem hiding this comment.
The ! not is missing. Also, why is a static_cast<bool> better?
There was a problem hiding this comment.
From ais type 6 documentation, it should not be negated !:
| 70 | 1 | Retransmit flag | retransmit | b | 0 = no retransmit (default) 1 = retransmitted |
|---|
The decoding of the messages type 6 from the tests:
{
'nmea': ['!AIVDM,1,1,,A,63m95T8uBK:0044@00P,2*7A'],
'result': {
'id': 6,
'repeat_indicator': 0,
'mmsi': 257050000,
'seq': 2,
'mmsi_dest': 257060000,
'retransmit': True,
'spare': 0,
'dac': 1,
'fi': 1,
'ack_dac': 64,
'msg_seq': 1,
'spare2': 0}},
{
'nmea': ['!AIVDM,1,1,,B,65@<;:1inW@h0480J0,4*60'],
'result': {
'id': 6,
'repeat_indicator': 0,
'mmsi': 352521000,
'seq': 0,
'mmsi_dest': 477535500,
'retransmit': True,
'spare': 0,
'dac': 1,
'fi': 2,
'req_dac': 1,
'req_fi': 40}}are not correct, as confirmed by decoding the AIS messages for instance here.
Therefore changed tests that are affected + this line.
@schwehr reverted to not using static_cast<bool>.
There was a problem hiding this comment.
Please use original source documentation for AIS message 6. I certainly could have been wrong, but unless you quote an official standard like ITU M.1371, I am not going to change it. gpsd aivdm docs were written by ESR with feedback from me before ITU M.1371 was freely available. I had a purchased copy; ESR did not.
So, I believe that I set this up as a bool called retransmit based on discussions at RTCM SC121 back in the 2007-2009 time frame.
3.4 Message 6: Addressed binary message; table 54:
Retransmit flag 1 Retransmit flag should be set upon retransmission: 0 = no retransmission =
default; 1 = retransmitted
I took the flag as a should retransmit.
No matter the correct answer, changing it in this pr is out of scope. Please don't do it here.
There was a problem hiding this comment.
@schwehr you are right, topic is out of scope for this PR. Will use original source doc in another issue. 👍
Reverted.
src/libais/ais_py.cpp
Outdated
| // TODO(schwehr): AIS_FI_6_1_30_TEXT. | ||
| case AIS_FI_6_1_32_TIDAL_WINDOW: // IMO Circ 289 | ||
| status = ais6_1_32_append_pydict(nmea_payload, dict, pad); | ||
| case AIS_DAC_1_INTERNATIONAL: // IMO. |
There was a problem hiding this comment.
ok. back to original indenting.
test/testutils.py
Outdated
| @@ -47,6 +47,8 @@ def DictDiff(a, b): | |||
| def Compare(x, y): | |||
| if x == y: | |||
| return True | |||
| if x in [None, 'nan'] and y in [None, 'nan']: | |||
There was a problem hiding this comment.
Why do you need this? And that says that None and 'nan' are ==. Is that really what we want?
There was a problem hiding this comment.
Modified because testTypeExamples fails. Not ideal. Reverted.
|
@schwehr would love to close this PR, could have a second look at it, and close ? |
This PR to solve #194