开云体育

  1. Jmriusers
  2. Topics

Locked JMRI and DCC++: Exception with Simple Programmer in Ops Accessory Byte/Extended Byte mode #dccpp #4-18

 

I am running JMRI 4.18+R37ad3d0 with Java 11.0.6 in Ubuntu, and DCC++ Basestation. When using Simple Programmer in Ops Accessory Byte or Ops Accessory Extended Byte, I get the following error:

2020-02-09 22:56:29,637 ptionhandler.UncaughtExceptionHandler ERROR - Uncaught Exception caught by jmri.util.exceptionhandler.UncaughtExceptionHandler [AWT-EventQueue-0]
java.lang.NullPointerException
at jmri.jmrix.dccpp.DCCppCommandStation.sendPacket(DCCppCommandStation.java:176)
at jmri.implementation.AccessoryOpsModeProgrammerFacade.writeCV(AccessoryOpsModeProgrammerFacade.java:174)
at jmri.jmrit.simpleprog.SimpleProgFrame.writePushed(SimpleProgFrame.java:223)
at jmri.jmrit.simpleprog.SimpleProgFrame$2.actionPerformed(SimpleProgFrame.java:66)
The DCC++ Traffic Monitor shows nothing. Alas I am not familiar with Java to identify what and where exactly communication between JMRI and DCC++ breaks.

Can it be the problem with DCC++ Basestation? As far as I understand, JMRI uses DCC++ command 'M' for Extended Accessory packets.
 

tnt23,

On 10 Feb 2020, at 7:06 AM, tnt23 <tim.tashpulatov@...> wrote:

I am running JMRI 4.18+R37ad3d0 with Java 11.0.6 in Ubuntu, and DCC++ Basestation. When using Simple Programmer in Ops Accessory Byte or Ops Accessory Extended Byte, I get the following error:
I looked at this at one stage, studied the DCC++ code and found that there's a maximum DCC packet length defined in the DCC++ code and that its one (or possibly two) bytes too short for NMRA S9.2.1 Ops Accessory Byte or Ops Accessory Extended Byte commands.

I came to the conclusion that, in order to solve the problem, the DCC++ code would have to be recompiled with this defined value changed.

But since I don't have the required Arduino hardware, nor the familiarity with recompiling the DCC++ code, plus the fact that there's nothing we can do in the JMRI code to work around it, I put the problem aside.

Dave in Australia
 

Dave,

Thank you for pointing this out. I too suspect the issue is with DCC++ not being updated for a good few years now.

I wouldn't mind fixing the DCC++ code, especially if it boils down to just increasing the maximum DCC packet length.
(Pushing the changes to DCC++ 'official' repository would be another story).

Regards

Tim
 

开云体育

Tim,

On 10 Feb 2020, at 5:32 PM, tnt23 <tim.tashpulatov@...> wrote:

Thank you for pointing this out. I too suspect the issue is with DCC++ not being updated for a good few years now.

I wouldn't mind fixing the DCC++ code, especially if it boils down to just increasing the maximum DCC packet length.
(Pushing the changes to DCC++ 'official' repository would be another story).

We need to be able to send 6 byte packets.

In the DCC++ code version I was looking at:

- Documentation at lines 482 ff of SerialCommand.cpp needs changing.

- The code lines that need changing are at line 189 onwards of PacketRegister.cpp:

void RegisterList::writeTextPacket(char *s) volatile{
??
? int nReg;
? byte b[6];
? int nBytes;
? volatile RegisterList *regs;
? ??
? nBytes=sscanf(s,"%d %x %x %x %x %x",&nReg,b,b+1,b+2,b+3,b+4)-1;
??
? if(nBytes<2 || nBytes>5){ ? ?// invalid valid packet
? ? INTERFACE.print("<mInvalid Packet>");
? ? return;
? }

A few changes needed there. There could be other implications of this change...


Dave in Australia
 

开云体育

Tim,

Sorry, resending with corrections:

On 10 Feb 2020, at 5:32 PM, tnt23 <tim.tashpulatov@...> wrote:

Thank you for pointing this out. I too suspect the issue is with DCC++ not being updated for a good few years now.

I wouldn't mind fixing the DCC++ code, especially if it boils down to just increasing the maximum DCC packet length.
(Pushing the changes to DCC++ 'official' repository would be another story).

We need to be able to send 6 byte packets.

In the DCC++ code version I was looking at:

- Documentation at lines 482 ff of SerialCommand.cpp needs changing.

- The code lines that need changing are at line 189 onwards of PacketRegister.cpp:

///////////////////////////////////////////////////////////////////////////////

void RegisterList::writeTextPacket(char *s) volatile{
??
? int nReg;
? byte b[6];
? int nBytes;
? volatile RegisterList *regs;
? ??
? nBytes=sscanf(s,"%d %x %x %x %x %x",&nReg,b,b+1,b+2,b+3,b+4)-1;
??
? if(nBytes<2 || nBytes>5){ ? ?// invalid valid packet
? ? INTERFACE.print("<mInvalid Packet>");
? ? return;
? }
? ? ? ? ?
? loadPacket(nReg,b,nBytes,0,1);
? ??
} // RegisterList::writeTextPacket()
??
///////////////////////////////////////////////////////////////////////////////


A few changes needed there. There could be other implications of this change...


Dave in Australia
 

Dave,

I have tweaked the RegisterList::writeTextPacket to accomodate for 6-byte packets, and amended the?RegisterList::loadPacket along with Packet structure's buf size. I can now see 6-byte packets in my DCC reception code (debugging custom decoder software).

However this did not make JMRI any happier. It still throws exception when trying to send out Ops Accessory Byte or Ops Accessory Extended Byte. And this puzzles me, here's why: as far as I understand, the 'M' command which JMRI uses to send raw packets to DCC++ Base Station would only complain in case of incorrect packet length (less than 2 bytes or more than 6 bytes). If a correct number of parameter is given, 'M' command would never report anything back, at least it does not when I use 'M' command manually.

So why would JMRI throw that exception if the DCC++ wouldn't complain?

Regards
Tim
 

开云体育

Tim,

On 11 Feb 2020, at 7:38 AM, tnt23 <tim.tashpulatov@...> wrote:

So why would JMRI throw that exception if the DCC++ wouldn't complain?

<>

if?(num_bytes?<?2?||?num_bytes?>?5)?return(null);

Guess you haven't got a checkout of the JMRI code?

I could do you a private build with that change later if need be...

Dave in Australia.
 

Dave,

<>
?
if?(num_bytes?<?2?||?num_bytes?>?5)?return(null);
This makes me think the DCCppMessage.java is worth updating for 6 byte packets, too :)

(It is probably worth nothing to note that DCC++ code, as it seems, will not spot extra bytes in 'M' message. The sscanf template used will always try to read up to 5 or 6 parameters, and will always return maximum number, 5 or 6, even if being fed 10 or 15 parameters)

Guess you haven't got a checkout of the JMRI code?
?
I could do you a private build with that change later if need be..
No, I did not checkout JMRI sources as I plain have no idea what to do with them in the first place :) Yet if you could build a test version for me that would be awesome. Thanks a bunch!

Regards
Tim
 

On another thought, though:

Extended Decoder Control Packet address for operations mode programming (clause 485 on page 10 of s-9.2.1) looks like this:

{preamble} 10AAAAAA 0 0AAA0AA1 0 (1110CCVV 0 VVVVVVVV 0 DDDDDDDD) 0 EEEEEEEE 1
Of these 6 bytes, last checksum byte does not need to be passed to DCC++ as it is calculated internally by DCC++ code when forming the packet. This means that 'M' command should be passing 5 bytes to DCC++, as follows:
<M 0 1 2 3 4 5>
, where the first 0 after M is Register number and should not count in num_bytes.

Is there a way to add parameters debugging output to console or log for the?makeWriteDCCPacketMainMsg?routine?

Regards
Tim
 

开云体育

Tim,

On 11 Feb 2020, at 5:59 PM, tnt23 <tim.tashpulatov@...> wrote:

Dave,

<>
?
if?(num_bytes?<?2?||?num_bytes?>?5)?return(null);
This makes me think the DCCppMessage.java is worth updating for 6 byte packets, too :)
Agreed, provided the DCC++code is updated as well

(It is probably worth nothing to note that DCC++ code, as it seems, will not spot extra bytes in 'M' message. The sscanf template used will always try to read up to 5 or 6 parameters, and will always return maximum number, 5 or 6, even if being fed 10 or 15 parameters)

That needs updating to scan the extra parameter and load it into the extra element of the array.


Guess you haven't got a checkout of the JMRI code?
?
I could do you a private build with that change later if need be..
No, I did not checkout JMRI sources as I plain have no idea what to do with them in the first place :) Yet if you could build a test version for me that would be awesome. Thanks a bunch!

Will have to be tomorrow. After dinner here and computers shut down due to storms.

Dave in Australia
 

开云体育

Tim,

On 11 Feb 2020, at 6:20 PM, tnt23 <tim.tashpulatov@...> wrote:

Of these 6 bytes, last checksum byte does not need to be passed to DCC++ as it is calculated internally by DCC++ code when forming the packet. This means that 'M' command should be passing 5 bytes to DCC++, as follows:
<M 0 1 2 3 4 5>
, where the first 0 after M is Register number and should not count in num_bytes.

The problem is that the NmraPacket class in JMRI creates and appends the checksum for every method (packet type) defined.
<>
We can't change that code without breaking every other system.

Packet lengths vary (they are not all 6 bytes).

Dropping the last byte of every M command changes the DCC++ specification as far as I can see and would break any non-JMRI software using the DCC++ system.

But too late in the day for this sort of thinking...

Dave in Australia
 

开云体育

Tim,

(It is probably worth nothing to note that DCC++ code, as it seems, will not spot extra bytes in 'M' message. The sscanf template used will always try to read up to 5 or 6 parameters, and will always return maximum number, 5 or 6, even if being fed 10 or 15 parameters)

That needs updating to scan the extra parameter and load it into the extra element of the array.

What does your modified code look like? Please show us.

My reading (I'm not a C++ speaker) of the code is that the sscanf ?function returns the number of successful matches*, not the number of parameters. So nBytes returns the actual number of parameters (minus 1 for the register parameter).

nBytes is the number of bytes specified to loadPacket?


*<>

Dave in Australia
 

Dave,

What does your modified code look like? Please show us.
Will share the changed pieces tonight once I am at my home computer.
?
My reading (I'm not a C++ speaker) of the code is that the sscanf ?function returns the number of successful matches*, not the number of parameters. So nBytes returns the actual number of parameters (minus 1 for the register parameter).
That is correct. My experience is based on feeding different variants of 'M' command to DCC++ Base Station via serial console, and watching the result. Started with '<M 0 1>', got '<mInvalid packet>' response in accordance with the (unmodified original) code:

nBytes=sscanf(s,"%d %x %x %x %x %x",&nReg,b,b+1,b+2,b+3,b+4)-1;
?
if(nBytes<2 || nBytes>5){ // invalid valid packet
INTERFACE.print("<mInvalid Packet>");
?

Next was the well-behaved '<M 0 1 2 3 4 5>' which went through perfectly silent.

Then I tried '<M 0 1 2 3 4 5 6 7 8'> and did not get error response. By adding debug print of nBytes I learned it is always set to 6, and it is the number of parameters the "%d %x %x %x %x %x"?format string expects (1 decimal for register number, and 5 hexes for parameters). It appears sscanf() just ignores the rest of input once it successfully stuffes all variables in its list.

But I believe it is irrelevant at this point. More important is the knowledge that JMRI always passes the packet bytes to the DCC++ including last checksumbyte. I will need to think this over, too.

Regards
Tim






?
nBytes is the number of bytes specified to loadPacket?


?
 

开云体育

Tim,

To late at night for me to be thinking code. We want to minimise changes and possible breaking of existing code, JMRI and other.

Please change:

1) Documentation at lines 482 ff of SerialCommand.cpp needs extra parameter adding.

2) Change??PacketRegister.cpp, method?writeTextPacket?as below:
///////////////////////////////////////////////////////////////////////////////

void RegisterList::writeTextPacket(char *s) volatile{
??
? int nReg;
? byte b[7];
? int nBytes;
? volatile RegisterList *regs;
? ??
? nBytes=sscanf(s,"%d %x %x %x %x %x %x",&nReg,b,b+1,b+2,b+3,b+4,b+5)-1;
??
? if(nBytes<2 || nBytes>6){ ? ?// invalid packet
? ? INTERFACE.print("<mInvalid Packet>");
? ? return;
? }
? ? ? ? ?
? loadPacket(nReg,b,nBytes,0,1);
? ??
} // RegisterList::writeTextPacket()
??
///////////////////////////////////////////////////////////////////////////////

I'll change:
Line 2441 of??in the private build to be:
? ?if?(num_bytes?<?2?||?num_bytes?> 6)?return(null);

You can then test the result with your DCC++ hardware.

Dave in Australia
 

开云体育

Tim,

On 11 Feb 2020, at 8:50 PM, tnt23 <tim.tashpulatov@...> wrote:

Next was the well-behaved '<M 0 1 2 3 4 5>' which went through perfectly silent.

Then I tried '<M 0 1 2 3 4 5 6 7 8'> and did not get error response. By adding debug print of nBytes I learned it is always set to 6, and it is the number of parameters the "%d %x %x %x %x %x"?format string expects (1 decimal for register number, and 5 hexes for parameters). It appears sscanf() just ignores the rest of input once it successfully stuffes all variables in its list.

But you didn't try ?'<M 0 1 2 3>' . I don't think you'll get nBytes=6 when the number of supplied parameters are less than those in the format string and variables).
See <>

Please set the DCC++ code as per my crossed-in-delivery email.

Dave in Australia
 

Tim, Dave,

"By adding debug print of nBytes I learned it is always set to 6, and it is the number of parameters the "%d %x %x %x %x %x"?format string expects (1 decimal for register number, and 5 hexes for parameters). It appears sscanf() just ignores the rest of input once it successfully stuffes all variables in its list."

It is very easy to make this code a lot more robust so it catches the 'too many parameters' as an invalid packet. Instead of passing in the actual message, pass in a modified one with a known trailing character (any will do, say 'X'). So in the example the string going in would become:

"M 0 1 2 3 4 5 6X"

and the scanf string:

"%d %x %x %x %x %xX"

No difference in result for this message, but if you have that extra input as in "M 0 1 2 3 4 5 6 7 8", (again passed in with the appended 'X' as):

"M 0 1 2 3 4 5 6 7 8X"

Using scanf with the same "%d %x %x %x %x %xX" now fails, detecting invalid packet, because it cannot match the now required 'X'.

It may not be a good moment for this change as you want the smallest possible modification at this point, but it is easy defensive programming that can catch unexpected issues, and could be something to keep in mind for later.

Wouter

On Tue, 11 Feb 2020 at 09:50, tnt23 <tim.tashpulatov@...> wrote:
Dave,

What does your modified code look like? Please show us.
Will share the changed pieces tonight once I am at my home computer.
?
My reading (I'm not a C++ speaker) of the code is that the sscanf ?function returns the number of successful matches*, not the number of parameters. So nBytes returns the actual number of parameters (minus 1 for the register parameter).
That is correct. My experience is based on feeding different variants of 'M' command to DCC++ Base Station via serial console, and watching the result. Started with '<M 0 1>', got '<mInvalid packet>' response in accordance with the (unmodified original) code:

nBytes=sscanf(s,"%d %x %x %x %x %x",&nReg,b,b+1,b+2,b+3,b+4)-1;
?
if(nBytes<2 || nBytes>5){ // invalid valid packet
INTERFACE.print("<mInvalid Packet>");
?

Next was the well-behaved '<M 0 1 2 3 4 5>' which went through perfectly silent.

Then I tried '<M 0 1 2 3 4 5 6 7 8'> and did not get error response. By adding debug print of nBytes I learned it is always set to 6, and it is the number of parameters the "%d %x %x %x %x %x"?format string expects (1 decimal for register number, and 5 hexes for parameters). It appears sscanf() just ignores the rest of input once it successfully stuffes all variables in its list.

But I believe it is irrelevant at this point. More important is the knowledge that JMRI always passes the packet bytes to the DCC++ including last checksumbyte. I will need to think this over, too.

Regards
Tim






?
nBytes is the number of bytes specified to loadPacket?


?
 

For the record, here are changes I made to DCC++ BaseStation code for 6 byte packets.

PacketRegister.h:

struct Packet{
? byte buf[11]; // was 10?
? byte nBits;
}; // Packet
PacketRegister.cpp: in?RegisterList::loadPacket starting from Line 70:

if(nBytes==3){
? ? p->nBits=49;
? } else{
? ? buf[6]+=b[3]>>2;? ? ? ? ? ? ? ? ? // b[3], bits 7-2
? ? buf[7]=b[3]<<6;? ? ? ? ? ? ? ? ? ?// b[3], bit 1-0
? ? if(nBytes==4){
? ? ? p->nBits=58;
? ? } else{
? ? ? buf[7]+=b[4]>>3;? ? ? ? ? ? ? ? // b[4], bits 7-3
? ? ? buf[8]=b[4]<<5;? ? ? ? ? ? ? ? ?// b[4], bits 2-0
? ? ? if(nBytes==5){
? ? ? ? p->nBits=67;
? ? ? } else{
? ? ? ? buf[8]+=b[5]>>4;? ? ? ? ? ? ? // b[5], bits 7-4
? ? ? ? buf[9]=b[5]<<4;? ? ? ? ? ? ? ?// b[5], bits 3-0
//? ? ? ? p->nBits=76;
? ? ? ? if (nBytes == 6) {
? ? ? ? ? p->nBits = 76;
? ? ? ? } else {
? ? ? ? ? buf[9]+=b[6]>>5;? ? ? ? ? ? ? // b[6], bits 7-5
? ? ? ? ? buf[10]=b[6]<<3;? ? ? ? ? ? ? ?// b[6], bits 4-0
? ? ? ? ? p->nBits=85;
? ? ? ? }? ? ? ??
? ? ? } // >5 bytes
? ? } // >4 bytes
? } // >3 bytes
and in?RegisterList::writeTextPacket, starting from Line 203:

void RegisterList::writeTextPacket(char *s) volatile{
??
? int nReg;
? byte b[7];
? int nBytes;
? volatile RegisterList *regs;
? ??
? nBytes=sscanf(s,"%d %x %x %x %x %x %x",&nReg,b,b+1,b+2,b+3,b+4, b+5)-1;
?
//INTERFACE.print(nBytes);
??
? if(nBytes<2 || nBytes>6){? ? // invalid valid packet
? ? INTERFACE.print("<mInvalid Packet>");
? ? return;
? }
? ? ? ? ?
? loadPacket(nReg,b,nBytes,0,1);
? ??
} // RegisterList::writeTextPacket()
Sorry I failed to make myself clear regarding sscanf() behavior, certainly it will report correct number of arguments scanned for 2, 3, 4, and 5 cases. What I meant was that it would report nBytes=6 when there are more arguments passed to it than is noted in its format string, and thus RegisterList::writeTextPacket() won't report this as an error.

Regards
Tim
 

(sorry for polluting the thread)

I couldn't help but opened an issue with JMRI repository, to save a bit of traffic here.



Dave, if you will be making a custom build for me, could you please 1) add some logging there to dump the whole 'M' command, and 2) instead of increasing length condition from 5 to 6, just change the 'for' cycle in line 2445 to run until num_bytes-1? Thanks again.

Regards
Tim
 

开云体育

Tim,

Thanks for reminding me about the buffer structure implications. I now remember why I put it aside last time I looked, because I wasn't up with how tight memory was, etc...

After I've had coffee and discussed some NCE issues with a colleague, I'll look at the JMRI code.

Dave in Australia


On 12 Feb 2020, at 2:46 AM, tnt23 <tim.tashpulatov@...> wrote:

For the record, here are changes I made to DCC++ BaseStation code for 6 byte packets.

PacketRegister.h:

struct Packet{
? byte buf[11]; // was 10?
? byte nBits;
}; // Packet
PacketRegister.cpp: in?RegisterList::loadPacket starting from Line 70:

if(nBytes==3){
? ? p->nBits=49;
? } else{
? ? buf[6]+=b[3]>>2;? ? ? ? ? ? ? ? ? // b[3], bits 7-2
? ? buf[7]=b[3]<<6;? ? ? ? ? ? ? ? ? ?// b[3], bit 1-0
? ? if(nBytes==4){
? ? ? p->nBits=58;
? ? } else{
? ? ? buf[7]+=b[4]>>3;? ? ? ? ? ? ? ? // b[4], bits 7-3
? ? ? buf[8]=b[4]<<5;? ? ? ? ? ? ? ? ?// b[4], bits 2-0
? ? ? if(nBytes==5){
? ? ? ? p->nBits=67;
? ? ? } else{
? ? ? ? buf[8]+=b[5]>>4;? ? ? ? ? ? ? // b[5], bits 7-4
? ? ? ? buf[9]=b[5]<<4;? ? ? ? ? ? ? ?// b[5], bits 3-0
//? ? ? ? p->nBits=76;
? ? ? ? if (nBytes == 6) {
? ? ? ? ? p->nBits = 76;
? ? ? ? } else {
? ? ? ? ? buf[9]+=b[6]>>5;? ? ? ? ? ? ? // b[6], bits 7-5
? ? ? ? ? buf[10]=b[6]<<3;? ? ? ? ? ? ? ?// b[6], bits 4-0
? ? ? ? ? p->nBits=85;
? ? ? ? }? ? ? ??
? ? ? } // >5 bytes
? ? } // >4 bytes
? } // >3 bytes
 

Tim,
Any chance that you could send me a copy of your modified DCC++ library? I would save me having to copy and paste the above text and possibly mess everything up. I have a DCC++ base station of my own , as well as a cuple that I built for friends
Stefan

--
Stefan Bartelski

Home layout: The Blue Ridge Line, an HO representation of the L&N Etowah Old Line from Etowah to Elizabeth and the Marble Hill branch (Georgia Marble Railroad), set in 1986 (under construction)?
Modular Layout: Shoofly module of the Country RRoads Modular group

1 - 20 of 22