Update PacketRegister.cpp

[kwledbetter]

The culprit is the packet sequence that is used to trigger programming, e.g. in RegisterList::readCV in PacketRegister.cpp

loadPacket(0,resetPacket,2,3); // NMRA recommends starting with 3 reset packets loadPacket(0,bRead,3,5); // NMRA recommends 5 verfy packets loadPacket(0,resetPacket,2,1); // forces code to wait until all repeats of bRead are completed (and decoder begins to respond)

This is the original code from the BaseStation some decoders do not perform the acknowledgement because of the last resetPacket that is used to wait until all bRead Packets have been digested. I changed the resetPacket to another bRead Packet and all works fine.

loadPacket(0,resetPacket,2,3); // NMRA recommends starting with 3 reset packets loadPacket(0,bRead,3,5); // NMRA recommends 5 verify packets loadPacket(0,bRead,3,1); // forces code to wait until all repeats of bRead are completed (and decoder begins to respond)

Similar changes are made in the routines for WriteCVByte and WriteCVBit.

Fixes https://github.com/DCC-EX/BaseStation/issues/3

[kwledbetter]

First Atani thanks for the link, saved me a google search!

After examining and with my limited knowledge I think we should correct with above to be NMRA compliant. First I am assuming we're supporting direct mode and have structured as such, let me know if that's not true.

loadPacket(0,resetPacket,2,3); // NMRA recommends starting with 3 reset packets loadPacket(0,bRead,3,5); // NMRA recommends 5 verify packets loadPacket(0,bRead,3,6); // NMRA recommends 6 write or reset packets for decoder recovery time loadPacket(0,resetPacket,2,1); // Final reset packet

Alternatively I think since this is reading aka verifying then theoretically we should be able to this:

loadPacket(0,resetPacket,2,3); // NMRA recommends starting with 3 reset packets loadPacket(0,bRead,3,5); // NMRA recommends 5 verify packets loadPacket(0,resetPacket,2,6); // NMRA recommends 6 write or reset packets for decoder recovery time

I'm not sure which is better but would assume second would be quicker? I'm way over my code knowledge skis here but If above looks good I will amend the Write portions appropriately before final pull as think this gets this piece NMRA compliant

[atanisoft]

Your updated steps look fine but we need to check the ACKs between bRead and resetPacket being sent. I don't know off hand if there is a "wait for this packet to be sent fully" option or not.

[kwledbetter]

Yes agree and this is where my lack of knowledge comes in. I can see on the packet register.h tab:

define ACK_BASE_COUNT 100 // number of analogRead samples to take before each CV verify to establish a baseline current

define ACK_SAMPLE_COUNT 500 // number of analogRead samples to take when monitoring current after a CV verify (bit or byte) has been sent

define ACK_SAMPLE_SMOOTHING 0.2 // exponential smoothing to use in processing the analogRead samples after a CV verify (bit or byte) has been sent

define ACK_SAMPLE_THRESHOLD 60 // the threshold that the exponentially-smoothed analogRead samples (after subtracting the baseline current) must cross to establish ACKNOWLEDGEMENT

and the back on the .cpp tab

for(int j=0;j<ACK_BASE_COUNT;j++) base+=analogRead(CURRENT_MONITOR_PIN_PROG); base/=ACK_BASE_COUNT;

bRead[0]=0x74+(highByte(cv)&0x03); // set-up to re-verify entire byte bRead[2]=bValue;

loadPacket(0,resetPacket,2,3); // NMRA recommends starting with 3 reset packets loadPacket(0,bRead,3,5); // NMRA recommends 5 verfy packets loadPacket(0,resetPacket,2,1); // forces code to wait until all repeats of bRead are completed (and decoder begins to respond)

for(int j=0;j<ACK_SAMPLE_COUNT;j++){ c=(analogRead(CURRENT_MONITOR_PIN_PROG)-base)ACK_SAMPLE_SMOOTHING+c(1.0-ACK_SAMPLE_SMOOTHING); if(c>ACK_SAMPLE_THRESHOLD) d=1; }

if(d==0) // verify unsuccessful bValue=-1;

so I can tell he's taking 100 samples for a baseline then 500 after and then the threshold compares to see if there is a difference to acknowledge it. What I am unsure of is what the resetPacket actually does or how long it takes , so I am not sure how the ACK actually happens and if this is handled properly or not. Like I say I've exhausted my limited knowledge on this subject.

I updated the code to what I think is full complance. I have not tested this at all yet but think should work but would appreciate some verification from someone that knows what they are doing :)

[kwledbetter]

OK Mike, made all the changes you suggested and cleaned up some white space, comment spacing, etc. Again I have not tested this at all yet but wanted to get your suggestions incorporated.

[atanisoft]

OK Mike, made all the changes you suggested and cleaned up some white space, comment spacing, etc. Again I have not tested this at all yet but wanted to get your suggestions incorporated.

It looks pretty good now with one minor update needed.

[kwledbetter]

OK Quick Question in line 237 loadPacket(0,resetPacket,2,1); // Final reset packet completed (and decoder begins to respond)

Does thatneed to be there or since the next set of programming is doing the re-verification of the byte do we want only the reset on line on 265?

[atanisoft]

Does thatneed to be there or since the next set of programming is doing the re-verification of the byte do we want only the reset on line on 265?

That reset can likely be omitted since it is a batch operation (write and read/verify)

[FrightRisk]

1. How do we implement issues so we can track this as an issue as you mentioned above
2. How do we implement Github Actions to do what you suggested on the Trainboard @atanisoft

[atanisoft]

1. How do we implement issues so we can track this as an issue as you mentioned above

I've filed https://github.com/DCC-EX/BaseStation/issues/3 to track the issue in this PR.

2. How do we implement Github Actions to do what you suggested on the Trainboard

Lets track that separately, I'll see what I can come up with for a basic test case and we can expand on it for different configurations etc.