search

mrrwa / NmraDcc

NMRA Digital Command Control (DCC) Library
GNU Lesser General Public License v2.1
139 stars 54 forks source link

Reading of CV values does not work #29

Closed MicroBahner closed 4 years ago

MicroBahner commented 5 years ago

Hi Alex, I open a new issue, so we can discuss the acknowledgement problem here separately. As I wrote already, with the actual version of 'master' reading of CV values in servicemode is no longer possible.

Some further thoughts to RailCom - as it is defined in NMRA S-9.3.2. RailCom is a really big extension to standard DCC. It is far more than an 'advanced acknowledge'. Bit 3 in CV29 should be named as what it really is: CV29_RailCom_ENABLE . If this bit is set in CV29 the decoder must conform to NMRA S-9.3.2 . I don't think you can build such a decoder with the lib so far. I am even not sure if it is possible with an AVR UNO/Nano at all. The timing in the RailCom cutout ist quite critical. In my opinion you need a free HW UART and maybe a HW Timer too. We should inhibit setting this bit.

With a standard Decoder (without RailCom) and programming in ops mode (POM) there is no acknowledgement at all. Programming in system mode requires the 6ms current pulse as acknowlegement. The most easiest way to accomplish this is to call notifyCVAck() only if we are in servicemode. I implemented this in my fork of the lib and can make a pull request if you agree with this solution.

kiwi64ajs commented 4 years ago

Ah huh, I think I've just encountered the same issue with Direct Mode CV programming and my solution was to provide a function pointer to the ackFunction() so it can be specified by the calling function as a parameter. This should not solve the problem of not being able to read CVs via Direct Mode. I've just committed the change to master and bumped the version to 2.0.3 but not tagged it yet so the Arduino IDE will not see the new version.

I agree we need to specify mode for Advanced CV Ack but I don't have any experience with it yet, but the reason for the original acAdvancedCV was to avoid trying to send an Ack for Ops Mode programming, as that messed up things. It shouldn't do that now.

MicroBahner commented 4 years ago

Hi Alex, yes that looks good now. Reading of CV's in service mode works again. But I don't think the solution with a functionpointer can be adequate if we will be railcom capable in the future. It's not only another ack-function, it also has to be called under other conditions. And therefore I think we will need two completely different functions for the opsmode (railcom) and servicemode programming. And probaly we'll need at least one parameter for this advanced ack function to indicate what has to be sent back. But that's still a long way off. For now it works ;)