notifyDccSigOutputState not called

[timtashpulatov]

I am using release JMRI 4.18 and DCC++ Basestation to generate Extended Accessory packets. With NmraDccAccessoryDecoder_1 example it appears that notifyDccSigOutputState is not called.

The JMRI seems to generate correct packet:

JMRI:
[TX: M 0 80 71 02 F3]   Write DCC Packet Main Cmd: 
  Register: 0
  Packet: 80 71 02 F3

The packet seems to be received just fine, except for the last byte (perhaps the checksum gets zeroed while checking). Number '5' in debug output is the received message length:

NMRA DCC Example 1
Init Done
notifyDccMsg: 5:80 71 2 F3 0 
notifyDccMsg: 5:80 71 2 F3 0 
notifyDccMsg: 5:80 71 2 F3 0

Yet the notifyDccSigOutputState is never called.

[timtashpulatov]

I have discovered that the notification callbacks are called after length condition in line 1233 is changed to 5 instead of 4:

https://github.com/mrrwa/NmraDcc/blob/7819716ac09c9d2864e49e80b6991db51edbcce9/NmraDcc.cpp#L1233

Both old and new callbacks fire:

notifyDccMsg: 5:80 71 1 F0 0 
notifyDccSigOutputState: 65533,1
notifyDccSigState: 65533,1

[kiwi64ajs]

Is the packet format you’re referring to described in the section starting on line 436 of https://www.nmra.org/sites/default/files/s-9.2.1_2012_07.pdf https://www.nmra.org/sites/default/files/s-9.2.1_2012_07.pdf or is it another because that only shows a 4-byte packet?

I’m pretty sure this has been used to drive Signals Aspects based on the 5-bit value and there’s also a comment to the effect that some manufacturers didn’t limit themselves to just the 5-bits so we’ve already relaxed that constraint.

So this leads me to think that its working ok - but you may have found a bug, that quite possible and of course the NMRA spec are not the most robust and have numerous ambiguities that I know we’ve had to make a judgement-call on. In some cases of ambiguity I just did what JMRI did and moved on… ;)

Alex

On 31/01/2020, at 5:55 AM, timtashpulatov notifications@github.com wrote:

I have discovered that the notification callbacks are called after length condition in line 1233 is changed to 5 instead of 4:

https://github.com/mrrwa/NmraDcc/blob/7819716ac09c9d2864e49e80b6991db51edbcce9/NmraDcc.cpp#L1233 https://github.com/mrrwa/NmraDcc/blob/7819716ac09c9d2864e49e80b6991db51edbcce9/NmraDcc.cpp#L1233 Both old and new callbacks fire:

notifyDccMsg: 5:80 71 1 F0 0 notifyDccSigOutputState: 65533,1 notifyDccSigState: 65533,1 — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/mrrwa/NmraDcc/issues/32?email_source=notifications&email_token=AB5Y53OZOWPSLGJ5KEFBO3TRAMA77A5CNFSM4KNLSMO2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKLW7LA#issuecomment-580349868, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB5Y53JCLZXXJ5P47KHNX3LRAMA77ANCNFSM4KNLSMOQ.

[timtashpulatov]

Hi Alex, thanks for prompt reply.

Is the packet format you’re referring to described in the section starting on line 436 of https://www.nmra.org/sites/default/files/s-9.2.1_2012_07.pdf https://www.nmra.org/sites/default/files/s-9.2.1_2012_07.pdf or is it another because that only shows a 4-byte packet?

Yes that's the Extended Accessory Control Packet: {preamble} 0 10AAAAAA 0 0AAA0AA1 0 000XXXXX 0 EEEEEEEE 1

Upon a closer look I think I've rung the bell too early. JMRI sends the packet correctly, having 4 bytes in total, with fourth byte (F3 in this case) being Checksum:

80 71 02 F3

While on the decoder side, these 4 bytes are extended with fifth (0):

80 71 2 F3 0

which I have mistook for the checksum. It seems someone had alredy seen this issue with DCC++ Base Station code. Quoting Terry Bailey, https://groups.io/g/jmriusers/message/152167:

...there was a bug in the DCC++ code which was tagging an extra 0 byte on the end of the packet on the rails

So most likely the problem lies within DCC++ Base Station code, not in NmraDcc library. Sorry for the false alarm!

PS While we are at it, may I ask another question on Checksum processing without creating another issue? Currently, the received packet checksum is calculated at once after the whole packet is received:

for(uint8_t i = 0; i < DccRx.PacketCopy.Size; i++)
      xorValue ^= DccRx.PacketCopy.Data[i];

Perhaps a small increase in efficiency could be gained should this cycle be replaced by single XOR operation each time a new packet byte is received. So by the time DccRx.DataReady is true, the received packet checksum will be ready in xorValue for evaluation.

Regards Tim

[kiwi64ajs]

I think I know the problem.

I think when JMRI forms DCC packets using that tool it expects you to enter the packets without a checksum byte as it computes that and appends it. So try entering the raw dcc packets without the checksum byte and see if that works

Regards

Alex Shepherd

On 31/01/2020, at 8:30 PM, timtashpulatov notifications@github.com wrote:

 Hi Alex, thanks for prompt reply.

Is the packet format you’re referring to described in the section starting on line 436 of https://www.nmra.org/sites/default/files/s-9.2.1_2012_07.pdf https://www.nmra.org/sites/default/files/s-9.2.1_2012_07.pdf or is it another because that only shows a 4-byte packet?

Yes that's the Extended Accessory Control Packet: {preamble} 0 10AAAAAA 0 0AAA0AA1 0 000XXXXX 0 EEEEEEEE 1

Upon a closer look I think I've rung the bell too early. JMRI sends the packet correctly, having 4 bytes in total, with fourth byte (F3 in this case) being Checksum:

80 71 02 F3

While on the decoder side, these 4 bytes are extended with fifth (0):

80 71 2 F3 0

which I have mistook for the checksum. It seems someone had alredy seen this issue with DCC++ Base Station code. Quoting Terry Bailey, https://groups.io/g/jmriusers/message/152167:

...there was a bug in the DCC++ code which was tagging an extra 0 byte on the end of the packet on the rails

So most likely the problem lies within DCC++ Base Station code, not in NmraDcc library. Sorry for the false alarm!

PS While we are at it, may I ask another question on Checksum processing without creating another issue? Currently, the received packet checksum is calculated at once after the whole packet is received:

for(uint8_t i = 0; i < DccRx.PacketCopy.Size; i++) xorValue ^= DccRx.PacketCopy.Data[i]; Perhaps a small increase in efficiency could be gained should this cycle be replaced by single XOR operation each time a new packet byte is received. So by the time DccRx.DataReady is true, the received packet checksum will be ready in xorValue for evaluation.

Regards Tim

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

[timtashpulatov]

The packets sent to Accessory Decoder are generated by JMRI automatically whenever I select different aspects for the selected Signal Mast.

I will try sending DCC packets manually, so far I haven't succeeded as it seems JMRI does not like the 'M' command.

[kiwi64ajs]

Hi Tim,

Looks like our emails crossed… ;)

Good you’ve spotted the issue and that its in the DCC++ codebase. I’m pretty certain I’ve used these packets with my Digitrax system and JMRI using the Simple Signal Logic capability and the NMRA Signal Mast variant that generates the 5-bit Aspect and it has been working.

If the DCC++ system is adding that extra byte then it will mess-up the parsing as it is.

To be tolerant of this DCC++ bug we’d have to either parse the other bytes in the packet more to know that it was an extended accessory or maybe just check for length >= 4 bytes and ignore the extra byte. You could make this change locally to get you going again and see if it gets fixed in DCC++

HTH

Alex Shepherd

On 31/01/2020, at 8:30 PM, timtashpulatov notifications@github.com wrote:

Hi Alex, thanks for prompt reply.

Is the packet format you’re referring to described in the section starting on line 436 of https://www.nmra.org/sites/default/files/s-9.2.1_2012_07.pdf https://www.nmra.org/sites/default/files/s-9.2.1_2012_07.pdf https://www.nmra.org/sites/default/files/s-9.2.1_2012_07.pdf https://www.nmra.org/sites/default/files/s-9.2.1_2012_07.pdf or is it another because that only shows a 4-byte packet?

Yes that's the Extended Accessory Control Packet: {preamble} 0 10AAAAAA 0 0AAA0AA1 0 000XXXXX 0 EEEEEEEE 1

Upon a closer look I think I've rung the bell too early. JMRI sends the packet correctly, having 4 bytes in total, with fourth byte (F3 in this case) being Checksum:

80 71 02 F3

While on the decoder side, these 4 bytes are extended with fifth (0):

80 71 2 F3 0

which I have mistook for the checksum. It seems someone had alredy seen this issue with DCC++ Base Station code. Quoting Terry Bailey, https://groups.io/g/jmriusers/message/152167 https://groups.io/g/jmriusers/message/152167:

...there was a bug in the DCC++ code which was tagging an extra 0 byte on the end of the packet on the rails

So most likely the problem lies within DCC++ Base Station code, not in NmraDcc library. Sorry for the false alarm!

PS While we are at it, may I ask another question on Checksum processing without creating another issue? Currently, the received packet checksum is calculated at once after the whole packet is received:

for(uint8_t i = 0; i < DccRx.PacketCopy.Size; i++) xorValue ^= DccRx.PacketCopy.Data[i]; Perhaps a small increase in efficiency could be gained should this cycle be replaced by single XOR operation each time a new packet byte is received. So by the time DccRx.DataReady is true, the received packet checksum will be ready in xorValue for evaluation.

Regards Tim

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mrrwa/NmraDcc/issues/32?email_source=notifications&email_token=AB5Y53PTBJS6OJ6INMVVDRDRAPHQ7A5CNFSM4KNLSMO2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKNYQLY#issuecomment-580618287, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB5Y53KF4FLZRVKIABLZ2GLRAPHQ7ANCNFSM4KNLSMOQ.

[timtashpulatov]

Hi Alex,

I guess this issue can be closed. I haven't found where exactly DCC++ code makes this padding yet. So far I did just like you proposed, changed length in my local repository to keep me happy for now.

Many thanks!

Regards Tim

[timtashpulatov]

I think I know the problem. I think when JMRI forms DCC packets using that tool it expects you to enter the packets without a checksum byte as it computes that and appends it.

Hi Alex,

You were right, it was JMRI sending 'M' command with appended checksum, which DCC++ code then appended another checksum to. The issue has been raised (https://github.com/JMRI/JMRI/issues/8110), thank you for cooperation.

Regards Tim