Patch 1 - Bring code into NMRA Spec

[FrightRisk]

Just want to make sure this is ready to merge. Packets corrected and typos fixed

Fixes #3

[FrightRisk]

And I haven't looked to see if it was implemented anywhere else, but there is an addition at the bottom of this unit in their code that we have not put in. Another packet definition:

byte RegisterList::decoderresetPacket[3]={0x7F,0x08,0x77};

Perhaps they were going to try something else and didn't complete it? The only other reference for it that I found was where it was defined in PacketRegister.h as a new element of the RegisterList struct. (static byte decoderresetPacket[];)

[atanisoft]

Perhaps they were going to try something else and didn't complete it? The only other reference for it that I found was where it was defined in PacketRegister.h as a new element of the RegisterList struct. (static byte decoderresetPacket[];)

I'm not certain what it might be, I'd have to decode the hex to understand what it is sending but it doesn't look like a reset packet.

[FrightRisk]

Oh, and last item was that the fixes in that TB thread also changed these values in PacketRegister.h:

define ACK_SAMPLE_COUNT original 500, fix 250, // analogRead samples to take

define ACK_SAMPLE_THRESHOLD original 30, fix 5 // exponentially-smoothed

As an aside, I think these need to be in config.h , don't you think so things we might actually have to tinker with don't require sorting through multiple files? I'm torn between having just a simple config, but where something like this could be changed and break things, and having settings in the .h files for each unit.

[atanisoft]

define ACK_SAMPLE_COUNT original 500, fix 250, // analogRead samples to take

This is fine either way, it is just a count of analog read calls.

define ACK_SAMPLE_THRESHOLD original 30, fix 5 // exponentially-smoothed

The NMRA spec is explicit in that a positive ACK must be at least 60mA which would be calculated in the 0-1023 analog steps. With the various h-bridges providing different current sense values this should likely be tied to the h-bridge and not be hard coded as it is today. I would suggest tracking that as part of the same issue (#3) but split off of this PR.

I'm torn between having just a simple config, but where something like this could be changed and break things, and having settings in the .h files for each unit.

Less is more. The less a user has to modify to get their system up and running the more likely they will stick with it since it just works.

[kwledbetter]

Yes agree it should be in the board/shield configuration ultimately.

My suggestion would be lets push what we have now to dev so testing can be done.

We can open a new pull request to move the ACK_SAMPLE_THRESHOLD to sit with the board configuration. I'm indifferent on moving to 250 or 500, honestly shouldn't matter.