Accessory decoder changes incompatible to previous versions

MicroBahner commented 6 years ago

Hi Alex, I detected, that the actual version 1.4.4 is no longer compatible to previous releases regarding the accessory decoder. The callback functions changed, and the old one is no longer available. Now sketches don't work anymore without any error message. Is this really intended to be so?

Regards Franz-Peter

kiwi64ajs commented 6 years ago

Hi Franz-Peter,

On 27/02/2018, at 9:27 AM, Franz-Peter notifications@github.com wrote:

Hi Alex, I detected, that the actual version 1.4.4 is no longer compatible to previous releases regarding the accessory decoder. The callback functions changed, and the old one is no longer available. Now sketches don't work anymore without any error message. Is this really intended to be so?

Hmmm… This is the downside of using the “weak reference” mechanism as it will silently ignore your call-back function if it’s not got the right signature.

I haven’t released this version of the code to the Arduino IDE Library manager yet as I was hoping for some feedback from several people to make sure I’d not broken something - so I guess you’re providing that now.

The reason I reworked this and dropped support for the two call-backs:

extern void notifyDccAccState( uint16_t Addr, uint16_t BoardAddr, uint8_t OutputAddr, uint8_t State )
extern void notifyDccSigState( uint16_t Addr, uint8_t OutputIndex, uint8_t State)

is that upon further research into “Output Address Mode” I discovered the logic and approach was just plain wrong as I was mixing up "Board Addressing" and "Output Addressing” which upon reflection, makes no sense as you can’t have both at the same time, which is what notifyDccAccState() was kinda portraying.

If you look at the call-backs now you’ll see they either have Board Addressing or Output Addressing but not both as I had before.

I’ve hunted around to see if I can somehow mark the old function as deprecated or generate a warning somehow if you’ve defined a now unsupported call-back in user code, but I’ve not found anything.

The only thing I can think of to do is change the version to 2.x.x and add more text to the library description to draw attention to the fact that support for those two functions has been removed.

I also just realised that I’d not gone back into the examples and changed the call-back to use the new functions.

What do you think is the best thing to do?

Alex Shepherd

MicroBahner commented 6 years ago

Hi Alex, well, the missing warning or error message is definitely a problem of the (weak) attribute. I don't know a solution for this problem either. So we need another solution to warn the programmer about that.

I realized this problem of this new version when users of my 'DCC_Zubehoerdecoder' complained that it did not work. This accessory decoder ist designed for users who don't want ( or are not able ) to program a lot. It can be widely customized by only changing some 'const' definitions in a configuration file. The user usually does not have to change the code itself to adapt the decoder to his needs. Some users downloaded the NmraDcc lib directly from github 'master' instead of installing from the library manager. They had no chance to find the reason why it didn't work.

I have to change the decoder, so that it works with the new and the old version without having the user to look after the correct version. At least I need a way to distinguish between old and new Version - e.g. by means of a new #define - to create the correct decoder that fits to the installed nmraDcc library. Or may it be a solution to provide the old an new callbacks in parallel? I'm not quite sure about that and will give it some thought - or try it out ;) .

At least you should change the Version to 2.0.0. I think, if only the minor version number changes, no one expects that it might be incompatible to the previous Version. I saw that you already changed the readme, which will help a lot. But this readme isn't seen if you install the lib with the Library manager. A hint in the library.properties 'paragraph' section may be useful, as this text can be seen in the Library manager. Edit: Sorry, didn't see that you already edited the library.properties file.

Another possibility might be to provide the old functions in parallel and mark them as 'deprecated' and 'only for compatibility reasons' without any further documentation. The nmradcc.h file is now commented very well ( that's a really big improvement ). I don't think that new users of the lib will ever use those old 'undocumented' functions.

Finally one question to the new callbacks: You can change between 'Board Adressing' or 'Output Adress Mode' by means of changing CV29. I think the lib itself must distinguish between the two modes only if FLAGS_MY_ADDRESS_ONLY is set. The only difference is how the Decoder address in CV1/CV9 is interpreted. If FLAGS_MY_ADDRESS_ONLY is not set it is up to the user to distinguish between the two adressing modes. The DCC-packet itself is always the same - it's only a matter of interpretation. The old callbacks left it up to the user how to interpret it. I don't think it was really 'wrong'. Or is there something I forgot?

MicroBahner commented 6 years ago

Hi Alex, when doing some testing with my accessory decoder there were some points that seem to be inconsistent and a little bit weired for me: The call of getAddr() now returns a value one less than the address in CV1/9. That's really confusing, and I think it should be the same ( as it was before ). The decoder address is the value in CV1/CV9. This should be independent of the address mode ( decoder or output ) which only makes the interpretation of this address regarding the received packet different. from 'NMRA s-9.2.2_decoder_cvs': The values contained in CV1 [513] and CV9 [521] correspond to the bits in the Accessory Decoder packets as follows: Accessory-Output = (CV1 [513] + (CV9 [521] *256)) - 1

That a decoder address of 1 corresponds to an outputaddress of 0 in the DCC packet should be transparent to the sketch. What makes things even more complex ist the offset of 4, which is used by most command stations.

from 'NMRA s-9.2.2_decoder_cvs': Decoders using either type of addressing will respond to the same Accessory Decoder Control Packet when CV1 [513] = 1 and CV9 [521] = 0. The factory default value is 1

In my opinion this means with a decoder address of 1, there should be no difference between the two address modes.

But I think these two statements are a bit contradictory. The problem seems to be, that decoder addresses in CV1/9 should start with '1' while the addresses in the DCC packet start with '0'. In any case: a address of '1' in CV1/9 should correspond to a turnout address of '1' sent by the command station. Anything else would be very disturbing to the user. ROCO sends a packet with all address bits set to '0' in this case, which can be interpreted as output address '0' or as decoder address '0' with decoder output '0'. This fits to the first NMRA statement above. The others send a packet with all address bits set to '4' which could be interpreted as output address '4' or decoder address 1 and decoder output '0'. With the second statement above, this means an ouput address of '4' in the packet corresponds to a turnoutaddress of 1.

It does not really fit together. I think Version 1.4.2 wasn't too bad.

kiwi64ajs commented 6 years ago

Hi Franz-Peter,

On 1/03/2018, at 11:12 AM, Franz-Peter notifications@github.com wrote: Hi Alex, well, the missing warning or error message is definitely a problem of the (weak) attribute. I don't know a solution for this problem either. So we need another solution to warn the programmer about that.

I realized this problem of this new version when users of my 'DCC_Zubehoerdecoder' https://github.com/MicroBahner/DCC_Zubehoerdecoder complained that it did not work. This accessory decoder ist designed for users who don't want to ( or are not able ) to program a lot. It can be widely customized by only changing some 'const' definitions in a configuration file. The user usually does not have to change the code itself to adapt the decoder to his needs. Some users downloaded the NmraDcc lib directly from github 'master' instead of installing from the library manager. They had no chance to find the reason why it didn't work.

I have to change the decoder, so that it works with the new and the old version without having the user to look after the correct version. At least I need a way to distinguish between old and new Version - e.g. by means of a new #define - to create the correct decoder that fits to the installed nmraDcc library. Or may it be a solution to provide the old an new callbacks in parallel? I'm not quite sure about that and will give it some thought - or try it out ;) .

At least you should change the Version to 2.0.0. I think, if only the minor version number changes, no one expects that it might be incompatible to the previous Version. I saw that you already changed the readme, which will help a lot. But this readme isn't seen if you install the lib with the Library manager. A hint in the library.properties 'paragraph' section may be useful, as this text can be seen in the Library manager.

I have added to the library.properties 'paragraph’ but lets go for version 2.0.0 and I’ll make the warning more obvious… ;)

Another possibility might be to provide the old functions in parallel and mark them as 'deprecated' and 'only for compatibility reasons' without any further documentation. The nmradcc.h file is now commented very well ( that's a really big improvement ). I don't think that new users of the lib will ever use those old 'undocumented' functions.

I’ll try this approach as well. I’ll try and put the notifyDccAccState() and notifyDccSigState()

Finally one question to the new callbacks: You can change between 'Board Adressing' or 'Output Adress Mode' by means of changing CV29. I think the lib itself must distinguish between the two modes only if FLAGS_MY_ADDRESS_ONLY is set. The only difference is how the Decoder address in CV1/CV9 is interpreted. If FLAGS_MY_ADDRESS_ONLY is not set it is up to the user to distinguish between the two adressing modes. The DCC-packet itself is always the same - it's only a matter of interpretation. The old callbacks left it up to the user how to interpret it. I don't think it was really 'wrong'. Or is there something I forgot?

That’s probably an error but the NMRA DCC specs are so confusing in this area that its hard to know what is the expected behaviour. I’ll check the logic around FLAGS_MY_ADDRESS_ONLY.

I was trying to eliminate the need for the user to “interpret” anything but maybe I’m not “done” yet… ;)

Alex

MicroBahner commented 6 years ago

Hi Alex, I made some changes to the lib to make a suggestion regarding the addressing. I also added a define wich contains the version number. This makes it easier for the sketch to call the correct methods.

So, when I had a closer look to the changes you made and our discussion above, there are some additional questions/ideas I'd like to discuss:

You cached the decoder address to make getMyAddress() a little bit faster ( or is there another reason ? ). But it is really worth it? I think it introduces new possible errors. The effective address can change in many ways. I think these:
```
switch( CV )
{
case CV_ACCESSORY_DECODER_ADDRESS_LSB:  // Also same CV for CV_MULTIFUNCTION_PRIMARY_ADDRESS
case CV_ACCESSORY_DECODER_ADDRESS_MSB:
case CV_MULTIFUNCTION_EXTENDED_ADDRESS_MSB:
case CV_MULTIFUNCTION_EXTENDED_ADDRESS_LSB:
  DccProcState.myDccAddress = -1;   // Assume any CV Write Operation might change the Address
}
```
are not the only ones. What if CV29 is changed and the address mode changes ( bit 6 in accessory decoders ore bit 5 in multifunction decoders ). Does the caching really give much benefit?
The NMRA papers explicity allow a decoder to span several addresses. in NMRA 9.2.2: If an accessory decoder supports more than one sequential output the value in CV1 [513] will be the first output in the series

How about supporting this in the lib? An additional parameter in the init command could define how many addresses are spanned ( if FLAGS_MY_ADDRESS_ONLY is set).

I can send my suggestions as a pull request, if you want. But its still work in progress ;-)

Franz-Peter

kiwi64ajs commented 6 years ago

Hi Franz-Peter,

On 10/04/2018, at 7:54 AM, Franz-Peter notifications@github.com wrote:

Hi Alex, I made some changes to the lib to make a suggestion regarding the addressing. I also added a define wich contains the version number. This makes it easier for the sketch to call the correct methods.

So, when I had a closer look to the changes you made and our discussion above, there are some additional questions/ideas I'd like to discuss:

You cached the decoder address to make getMyAddress() a little bit faster ( or is there another reason ? ). But it is really worth it? I think it introduces new possible errors. The effective address can change in many ways. I think these: switch( CV ) { case CV_ACCESSORY_DECODER_ADDRESS_LSB: // Also same CV for CV_MULTIFUNCTION_PRIMARY_ADDRESS case CV_ACCESSORY_DECODER_ADDRESS_MSB: case CV_MULTIFUNCTION_EXTENDED_ADDRESS_MSB: case CV_MULTIFUNCTION_EXTENDED_ADDRESS_LSB: DccProcState.myDccAddress = -1; // Assume any CV Write Operation might change the Address } are not the only ones. What if CV29 is changed and the address mode changes ( bit 6 in accessory decoders ore bit 5 in multifunction decoders ). Does the caching really give much benefit?

EEPROM access is not immediate and if there is an EEPROM write operation happening then and subsequent EEPROM actions will block (for 1.8ms) which is bad and why I cache this value. Maybe we should cache some more as well but I kinda cache most of CV29 already which is the other important one.

How about we just add CV_29 (and any others you can think of) to the list of CVs that invalidates the address cache? Would that overcome your concerns?

The NMRA papers explicity allow a decoder to span several addresses. in NMRA 9.2.2: If an accessory decoder supports more than one sequential output the value in CV1 [513] will be the first output in the series How about supporting this in the lib? An additional parameter in the init command could define how many addresses are spanned ( if FLAGS_MY_ADDRESS_ONLY is set).

Yeah this behaviour is not so well defined - mostly because the NMRA specs are so ambiguous and hard to figure out (at the time I did most of this) really what should happen, so I’m open to suggestions…;)

I imagined a couple of scenarios:

1) Your decoder has a list of sequential addresses, so we could define a start and end or start and length of address group. The latter is probably better.

2) Your decoder has a list of non-sequential addresses, so internally we’d need to compare any incoming addresses to our list to see if we care about it.

Really my thinking around FLAGS_MY_ADDRESS_ONLY was for the NMRA Board Address type logic which was pretty well defined.

However as soon as you go into Signals and non-sequential addressing all the simplicity disappears. It’s not rocket science but deciding how to do something to handle this in an intuitive and generic way hasn’t materialised in my head yet. Hence the reason it is the way it is. As it is you can either use the FLAGS_MY_ADDRESS_ONLY with a Board address or do your own thing checking the Address in the notifyXXX call-back functions.

I can send my suggestions as a pull request, if you want. But its still work in progress ;-)

That would be good. I don’t profess to be the sole source of DCC Decoder wisdom and this is a community right… ;)

HTH

Alex Shepherd

MicroBahner commented 6 years ago

Hi Alex

How about we just add CV_29 (and any others you can think of) to the list of CVs that invalidates the address cache? Would that overcome your concerns?

Yes, that is what I did already ;) . I didn't notice, that writing to the EEPROM of course delays reading from the EEPROM too.

I was trying to eliminate the need for the user to “interpret” anything but maybe I’m not “done” yet… ;)

I think, that's not possible. The library should not be too restrictive, when interpreting the packets. In the meanwhile I had a closer look at the MOROP papers. These norms are based on the NMRA, but in some points they are enhanced. Unfortunately they are only available in german and french so far ( They are searching for someone to translate to english ). E.g. the extended accessory packet: the NMRA defines the 3rd byte as 000xxxx. MOROP uses all 8 bits. I think, the library should not mask the upper bits and leave it to the sketch to interpret this byte accordingly.

Really my thinking around FLAGS_MY_ADDRESS_ONLY was for the NMRA Board Address type logic which was pretty well defined.

Yes, and maybe its the better way to leave checking the address to the sketch in all other cases?

The more papers you read, the more obscure it seems to be - especially with output addressing. With extended accessory packets obviously there is only output addressing. Unfortunaltely I don't have any equipment that generates such packets. I wonder whether there is an offset in addressing just like the basic packets. Does signal address '1' generate a packet with packet address '0' or packet addres '4' ? If there is no such offset, the output address should be computed different between basic and extended accessory packets.

Franz-Peter

kiwi64ajs commented 6 years ago

I've committed your PR that restores one of the call-backs I had dropped. extern void notifyDccAccState( uint16_t Addr, uint16_t BoardAddr, uint8_t OutputAddr, uint8_t State )

I guess to be consistent we should restore the other call as well: extern void notifyDccSigState( uint16_t Addr, uint8_t OutputIndex, uint8_t State)

I'll leave this open to remind me...

MicroBahner commented 6 years ago

Hi Alex,

I guess to be consistent we should restore the other call as well:

Yes that's true. It doesn't make sense to restore only one of the two. I got it off my mind because I never used that callback.

kiwi64ajs commented 6 years ago

Ok yes. I’ll do that.

I’ve had quite a bit of interaction with someone else building a coach lighting decoder and from that I’m thinking of adding another check in the Dcc.init() method to first check the storage of CV 7&8 to see if they are 255 prior to being set which would indicate a fresh or erased chip and trigger the RestoreFactoryDefaults call-back.

What do you think?

Alex

Sent from my iPad

On 18/04/2018, at 8:40 AM, Franz-Peter notifications@github.com wrote:

Hi Alex,

I guess to be consistent we should restore the other call as well: Yes that's true. It doesn't make sense to restore only one of the two. I got it off my mind because I never used that callback.

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

gbglacier commented 6 years ago

That is not a good idea, basing resets on pre-supposed EEPROM contents. Use some explicit, non-ambiguous, assumption-less methods.

Also, are all these changes going to change the default accessory decoder addressing method via JMRI? If yes, I would advocate a strenuous no, leave the current mechanism the default. Otherwise you are going to potentially mess up a few thousand decoders coming on line. Best regards, Geoff Bunza

Sent from XFINITY Connect App

------ Original Message ------

From: Alex Shepherd To: mrrwa/NmraDcc Cc: Subscribed Sent: April 17, 2018 at 1:50 PM Subject: Re: [mrrwa/NmraDcc] Accessory decoder changes incompatible to previous versions (#15)

Ok yes. I’ll do that.

I’ve had quite a bit of interaction with someone else building a coach lighting decoder and from that I’m thinking of adding another check in the Dcc.init() method to first check the storage of CV 7&8 to see if they are 255 prior to being set which would indicate a fresh or erased chip and trigger the RestoreFactoryDefaults call-back.

What do you think?

Alex

Sent from my iPad

On 18/04/2018, at 8:40 AM, Franz-Peternotifications@github.comwrote:

Hi Alex,

I guess to be consistent we should restore the other call as well: Yes that's true. It doesn't make sense to restore only one of the two. I got it off my mind because I never used that callback.

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

— 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/15#issuecomment-382140746), ormute the thread(https://github.com/notifications/unsubscribe-auth/APID0nKgBBmuGeY6h10kxfMDqhtLeVoaks5tplWFgaJpZM4ST32j).

kiwi64ajs commented 6 years ago

Hi Geoff,

On 18/04/2018, at 9:01 AM, Geoff Bunza notifications@github.com wrote:

That is not a good idea, basing resets on pre-supposed EEPROM contents. Use some explicit, non-ambiguous, assumption-less methods.

Hmmm… Ok

The use-case is quite specific, where the two Read-Only CVs CV_VERSION_ID (CV7) = 255 and CV_MANUFACTURER_ID (CV8) = 255 means the EEPROM has been erased and should optionally be reset to the Factory Defaults, if that call-back is defined.

However to not risk breaking existing code, I can make this new proposed behaviour dependant on passing in a new FLAGS_AUTO_FACTORY_DEFAULT option into the Dcc.init() method, that enables the check and optional call to notifyCVResetFactoryDefault() if it is defined and if the 2 CV in EEPROM = 255.

I’ve committed this change in GitHub so you can take a look and comment.

Also, are all these changes going to change the default accessory decoder addressing method via JMRI? If yes, I would advocate a strenuous no, leave the current mechanism the default. Otherwise you are going to potentially mess up a few thousand decoders coming on line.

I expect not, unless it’s actually broken now. I need to double-check the NMRA DCC Specs against the current implementation and run some more test cases through it to be sure.

For now I’ve NOT bumped the released TAG in the Git Repo, so the version of the library that the Arduino IDE Library Manager downloads is still 1.4.2 which has none of these changes, as I’m not sure we are “there yet”…

HTH

Alex Shepherd

MicroBahner commented 6 years ago

Hi Alex and Geoff,

On 18/04/2018, at 9:01 AM, Geoff Bunza @.***> wrote:

That is not a good idea, basing resets on pre-supposed EEPROM contents. Use some explicit, non-ambiguous, assumption-less methods. Hmmm… Ok

The use-case is quite specific, where the two Read-Only CVs CV_VERSION_ID (CV7) = 255 and CV_MANUFACTURER_ID (CV8) = 255 means the EEPROM has been erased and should optionally be reset to the Factory Defaults, if that call-back is defined.

I go along with Geoff and would not do too much specific stuff in the lib. Its easy in the Sketch to decide whether the EEPROM has to be initialized or not. But if you make it dependent on a special flag ...

Also, are all these changes going to change the default accessory decoder addressing method via JMRI? If yes, I would advocate a strenuous no, leave the current mechanism the default. Otherwise you are going to potentially mess up a few thousand decoders coming on line.

I expect not, unless it’s actually broken now. I need to double-check the NMRA DCC Specs against the current implementation and run some more test cases through it to be sure.

The addressing method should now be the same as in 1.4.2. The main difference is in getMyAddr, as in 1.4.2 this didn't work in OuputMode for adresses greater than 255. But because of the many code changes and additions it should be tested thoroughly.

I did some tests with JMRI and a dcc++ base station. I didn't see any problems with the addressing method so far. But I found another problem: When using FLAGS_OUTPUT_ADDRESS_MODE and FLAGS_MY_ADDRESS_ONLY together, I never got a match. Obviously it is caused by this code (line 1114):

  if( ( DccProcState.Flags & FLAGS_OUTPUT_ADDRESS_MODE ) && ( OutputAddress != getMyAddr() ) && ( OutputAddress < 2045 ) )
    return;
  else if( ( BoardAddress != getMyAddr() ) && ( BoardAddress < 511 ) )
    return;
  DB_PRINT("execDccProcessor: Address Matched");

If the Outputaddress matches, the first if gets false, and then the else ist executed, where the Boardaddress is tested, what always fails if the address is >1.

The first if has to be split in two separated ifs to get it working:

            if( DccProcState.Flags & FLAGS_OUTPUT_ADDRESS_MODE ) {
              if ( OutputAddress != getMyAddr()  &&  OutputAddress < 2045 ) {
                //DB_PRINT(" eDP: OutputAddress: %d, myAddress: %d - no match", OutputAddress, getMyAddr() );
                return;
              }  
            } else {
              if( ( BoardAddress != getMyAddr() ) && ( BoardAddress < 511 ) ) {
                //DB_PRINT(" eDP: BoardAddress: %d, myAddress: %d - no match", BoardAddress, getMyAddr() );
                return;
              }
            }
            DB_PRINT("execDccProcessor: Address Matched");

Franz-Peter

EFA57 commented 6 years ago

Geoff,

I have been trying for about a week to send this to this thread. I am sure this is the wrong thread but here go's.

I am new to dcc ++.

I purchased a mega and downloaded & successfully compiled the dccpp BaseStation Sketch to it.

I successfully got the dccppcontroler working in Processing IDE and connected to the BaseStation although w/o a motor shield I can not do anything with it.

I assume that the output is to low a voltage to even connect to the "programming" track although I have not tried yet?

I would like to know so many things it is hard to start.

1) How do I use my android phone/tab to run/test my 1 DCC equipped Engine? A) I saw what I think is a Processing IDE for android and installed? APDE I downloaded & installed Engine Driver. B) Can I access via the USB/Computer connection until l add a WiFi or BT shield?

2) How can I change the layout. (on controller) I looked at the section where it is located in the sketch but don't know what I am looking at to change?

3) I created a Sketch that alternately blinks led`s that I wondered if I could have use sensors to turn on at grade crossings on the same aduiano mega board or will I need to dedicate the mega to the BaseStation?

Any help would be much appreciated!

Thanks,

Erich efa57@yahoo.com 847.341.1120

MicroBahner commented 6 years ago

Hi Alex,

I guess to be consistent we should restore the other call as well: extern void notifyDccSigState( uint16_t Addr, uint8_t OutputIndex, uint8_t State)

I'll leave this open to remind me...

I now restored it in my last pull request ;)

Franz-Peter

kiwi64ajs commented 6 years ago

Thanks for doing this. I’ve not gotten to any of this for several months. I’ll try and get some testing time soon and also ping Geoff Bunga to also test his code.

Alex

Sent from my iPad

On 21/06/2018, at 3:17 AM, Franz-Peter notifications@github.com wrote:

Hi Alex,

I guess to be consistent we should restore the other call as well: extern void notifyDccSigState( uint16_t Addr, uint8_t OutputIndex, uint8_t State)

I'll leave this open to remind me...

I now restored it in my last pull request ;)

Franz-Peter

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

MicroBahner commented 6 years ago

Hi Alex

I’ll try and get some testing time soon and also ping Geoff Bunga to also test his code.

that would be fine. I think there are no open issues left, and testing is the most important thing to do now. I tested with a DCC++ commandstation and JMRI. I also enhanced DCC++ a little bit, to create extended accessory packets and accessory ops mode programming packets by means of a serial Terminal. The more independent tests, the better it is.

gbglacier commented 6 years ago

Working on this now.
Best regards,
Geoff

On 6/22/2018 2:01 AM, Franz-Peter
  wrote:

  Hi Alex

    I’ll try and get some testing time soon and also ping Geoff
      Bunga to also test his code.

  that would be fine. I think there are no open issues left, and
    testing is the most important thing to do now. I tested with a
    DCC++ commandstation and JMRI. I also enhanced DCC++ a little
    bit, to create extended accessory packets and accessory ops mode
    programming packets by means of a serial Terminal.
    The more independent tests, the better it is.
  —
    You are receiving this because you commented.
    Reply to this email directly, view it on GitHub, or mute the thread.
  {"@context":"http://schema.org","@type":"EmailMessage","potentialAction":{"@type":"ViewAction","target":"https://github.com/mrrwa/NmraDcc/issues/15#issuecomment-399374016","url":"https://github.com/mrrwa/NmraDcc/issues/15#issuecomment-399374016","name":"View Issue"},"description":"View this Issue on GitHub","publisher":{"@type":"Organization","name":"GitHub","url":"https://github.com"}}
  {"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/mrrwa/NmraDcc","title":"mrrwa/NmraDcc","subtitle":"GitHub repository","main_image_url":"https://assets-cdn.github.com/images/email/message_cards/header.png","avatar_image_url":"https://assets-cdn.github.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/mrrwa/NmraDcc"}},"updates":{"snippets":[{"icon":"PERSON","message":"@MicroBahner in #15: Hi Alex\r\n\u003e I’ll try and get some testing time soon and also ping Geoff Bunga to also test his code.\r\n\r\nthat would be fine. I think there are no open issues left, and testing is the most important thing to do now. I tested with a DCC++ commandstation and JMRI. I also enhanced DCC++ a little bit, to create extended accessory packets and accessory ops mode programming packets by means of a serial Terminal.\r\nThe more independent tests, the better it is.\r\n"}],"action":{"name":"View Issue","url":"https://github.com/mrrwa/NmraDcc/issues/15#issuecomment-399374016"}}}
  {

"@type": "MessageCard", "@context": "http://schema.org/extensions", "hideOriginalBody": "false", "originator": "AF6C5A86-E920-430C-9C59-A73278B5EFEB", "title": "Re: [mrrwa/NmraDcc] Accessory decoder changes incompatible to previous versions (#15)", "sections": [ { "text": "", "activityTitle": "Franz-Peter", "activityImage": "https://assets-cdn.github.com/images/email/message_cards/avatar.png", "activitySubtitle": "@MicroBahner", "facts": [

] } ], "potentialAction": [ { "name": "Add a comment", "@type": "ActionCard", "inputs": [ { "isMultiLine": true, "@type": "TextInput", "id": "IssueComment", "isRequired": false } ], "actions": [ { "name": "Comment", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"IssueComment\",\n\"repositoryFullName\": \"mrrwa/NmraDcc\",\n\"issueId\": 15,\n\"IssueComment\": \"{{IssueComment.value}}\"\n}" } ] }, { "name": "Close issue", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"IssueClose\",\n\"repositoryFullName\": \"mrrwa/NmraDcc\",\n\"issueId\": 15\n}" }, { "targets": [ { "os": "default", "uri": "https://github.com/mrrwa/NmraDcc/issues/15#issuecomment-399374016" } ], "@type": "OpenUri", "name": "View on GitHub" }, { "name": "Unsubscribe", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"MuteNotification\",\n\"threadId\": 307199395\n}" } ], "themeColor": "26292E" }

--

    Geoff Bunza
    gbglacier@comcast.net
    scalemodelanimation.com

gbglacier commented 6 years ago

Hi,
Here's a quick testing update: It looks like the
  timing of the main loop has slowed down 
  and/or has become variable -- I'm not 100% sure which. It looks
  like it is not as consistent as it was in 
  the last major release since I updated the SMA examples (I'm not
  sure how far back that was). The net
  effect is that I have had to modify internal parameters to the
  decoder set.
  I am adding the last major functionality which appeared in the
  March 2017 Model Railroad Hobbyist
  magazine article, which includes audio and stepper control
  variants.
  I have already added long addressing for both mobile and accessory
  decoder variations for all examples.
  I have to write up the release notes and finish the complete set
  of hardware configuration tests.
  Baseline functionality looks consistently good.

  Has anyone done testing on the Teensy 3.2 and/or 3.6? I will
  likely use that in a near future project.

  Have fun!  :-)
  Best regards,
  Geoff Bunza      <--- Note Correct Spelling Please

On 6/22/2018 2:01 AM, Franz-Peter
  wrote:

  Hi Alex

    I’ll try and get some testing time soon and also ping Geoff
      Bunga to also test his code.

  that would be fine. I think there are no open issues left, and
    testing is the most important thing to do now. I tested with a
    DCC++ commandstation and JMRI. I also enhanced DCC++ a little
    bit, to create extended accessory packets and accessory ops mode
    programming packets by means of a serial Terminal.
    The more independent tests, the better it is.
  —
    You are receiving this because you commented.
    Reply to this email directly, view it on GitHub, or mute the thread.
  {"@context":"http://schema.org","@type":"EmailMessage","potentialAction":{"@type":"ViewAction","target":"https://github.com/mrrwa/NmraDcc/issues/15#issuecomment-399374016","url":"https://github.com/mrrwa/NmraDcc/issues/15#issuecomment-399374016","name":"View Issue"},"description":"View this Issue on GitHub","publisher":{"@type":"Organization","name":"GitHub","url":"https://github.com"}}
  {"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/mrrwa/NmraDcc","title":"mrrwa/NmraDcc","subtitle":"GitHub repository","main_image_url":"https://assets-cdn.github.com/images/email/message_cards/header.png","avatar_image_url":"https://assets-cdn.github.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/mrrwa/NmraDcc"}},"updates":{"snippets":[{"icon":"PERSON","message":"@MicroBahner in #15: Hi Alex\r\n\u003e I’ll try and get some testing time soon and also ping Geoff Bunga to also test his code.\r\n\r\nthat would be fine. I think there are no open issues left, and testing is the most important thing to do now. I tested with a DCC++ commandstation and JMRI. I also enhanced DCC++ a little bit, to create extended accessory packets and accessory ops mode programming packets by means of a serial Terminal.\r\nThe more independent tests, the better it is.\r\n"}],"action":{"name":"View Issue","url":"https://github.com/mrrwa/NmraDcc/issues/15#issuecomment-399374016"}}}
  {

"@type": "MessageCard", "@context": "http://schema.org/extensions", "hideOriginalBody": "false", "originator": "AF6C5A86-E920-430C-9C59-A73278B5EFEB", "title": "Re: [mrrwa/NmraDcc] Accessory decoder changes incompatible to previous versions (#15)", "sections": [ { "text": "", "activityTitle": "Franz-Peter", "activityImage": "https://assets-cdn.github.com/images/email/message_cards/avatar.png", "activitySubtitle": "@MicroBahner", "facts": [

] } ], "potentialAction": [ { "name": "Add a comment", "@type": "ActionCard", "inputs": [ { "isMultiLine": true, "@type": "TextInput", "id": "IssueComment", "isRequired": false } ], "actions": [ { "name": "Comment", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"IssueComment\",\n\"repositoryFullName\": \"mrrwa/NmraDcc\",\n\"issueId\": 15,\n\"IssueComment\": \"{{IssueComment.value}}\"\n}" } ] }, { "name": "Close issue", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"IssueClose\",\n\"repositoryFullName\": \"mrrwa/NmraDcc\",\n\"issueId\": 15\n}" }, { "targets": [ { "os": "default", "uri": "https://github.com/mrrwa/NmraDcc/issues/15#issuecomment-399374016" } ], "@type": "OpenUri", "name": "View on GitHub" }, { "name": "Unsubscribe", "@type": "HttpPOST", "target": "https://api.github.com", "body": "{\n\"commandName\": \"MuteNotification\",\n\"threadId\": 307199395\n}" } ], "themeColor": "26292E" }

--

    Geoff Bunza
    gbglacier@comcast.net
    scalemodelanimation.com

kiwi64ajs commented 6 years ago

I've just committed Franz-Peter's changes into the NmraDcc/master branch

mrrwa / NmraDcc

Accessory decoder changes incompatible to previous versions #15