Bug in Example CmdrArduino_accessory.ino

[liamoreilly]

I believe there is a bug with the translation of accessory decoder addresses in the example.

The following two lines of code use incorrect calculations in the arguments:

• dps.setBasicAccessory((address >> 2) + 1, ((address - 1) & 0x03) << 1);
• dps.unsetBasicAccessory((address >> 2) + 1, ((address - 1) & 0x03) << 1);

I believe the correct calculations and hence the corrected lines are:

• dps.setBasicAccessory(((address - 1) >> 2) + 1, ((address - 1) & 0x03));
• dps.unsetBasicAccessory(((address - 1) >> 2) + 1, ((address - 1) & 0x03));

I hope this can be fixed and changed in the codebase; or at least help others who run into trouble trying to control accessory decoders.

[UncannyBingo]

This may be a bug. TBH, I'm a bit rusty on accessory addresses, and I have never actually exercised this code, owning no accessory decoders :D Could you give me some worked examples that show the error explicitly?

[liamoreilly]

Hi,

Certainly. I have worked out some examples but they are on my office desk. I will post them Monday.

I'm not 100% up on DCC addesses, but with the modifications my Hornby R8247 accessory decoder successfully controls turnout motors. Having someone double check the calculations would be great.

[liamoreilly]

Here are examples.

I have created a table. The first Column "Accessory Address" is the address of the accessory as it appears to the DCC user. The second column is the first argument to the setBasicAccessory function, while the third is the second argument to the function. The fourth and fifth columns are the same except with my modification to the calculations.

Accessory Address	Real DCC Address Original	Function Part Original	Real DCC Address Modified	Function Part Modified
1	1	0	1	0
2	1	2	1	1
3	1	4	1	2
4	2	6	1	3
5	2	0	2	0
6	2	2	2	1
7	2	4	2	2
8	3	6	2	3
9	3	0	3	0
10	3	2	3	1
11	3	4	3	2
12	4	6	3	3
13	4	0	4	0
14	4	2	4	1
15	4	4	4	2
16	5	6	4	3

As you can see for the accessory address 0 both sets of calculations produce 1, 0. However, the original calculations then are a bit off. For example, for accessory address 3 the original calculations yields 1,4. Now this triggers the same point motor on my setup as accessory address 1 does. I believe this is because the function part (4) becomes 0 with modulo 4 arithmetic.

If you now look at the row for accessory address 4 you will see that the function part in the original calculation is 6. This seems incorrect.

I believe that the accessory addresses are in blocks of 4. Group 1 is accessory addresses 1,2,3,4. Group 2 is 5,6,7,8. Group 3 is 9,10,11,12. When you use an accessory address, it needs to be translated to the real address (i.e., ther group) and then the index of the list. So accessory address 7 is really group 2 and index 2. Now if you look at my calculations in the table above you will see this matches.

I hope this explanation helps and makes sense - but I feel it probably doesnt.

[liamoreilly]

I should say this works for my Hornby R8247 accessory decoder, but I am not sure if the addresses work like this in general for all accessory decoders.

[johnnylingo]

It's much easier for me to understand using this code, rather than bitshifting. It may actually take a couple more clock cycles, but for as often as it happens, not a big deal.
dps.setBasicAccessory(((funct + 3) / 4), ((funct + 3) % 4));