steveohara / j2mod

Enhanced Modbus library implemented in the Java programming language
Apache License 2.0
265 stars 111 forks source link

Listen to one specific UnitID with serial RTU slave? #62

Closed NogginBops closed 6 years ago

NogginBops commented 6 years ago

I'm trying to write an application that is supposed to emulate a slave on a specific address. So far I have gotten the program to receive data through the serial port and it sends back som messages to the master. But I sends a lot of exception messages because it receives messages ment for another unit. This is not good for the master program as it only expects one answer but gets a response from two units.

Is there a way to stop this behavior? To create the slave I do: ModbusSlave slave = ModbusSlaveFactory.createSerialSlave(params); I create a ProcessImageImplementation like this: And add it to the slave like this:

Is there something that I am doing wrong, something about the library I am using in a wrong way?

Otherwise this is a very good feature to have as it allows for simulating one of many devices on a network.

steveohara commented 6 years ago

I think logically there must be something very wrong here. A Modbus Slave should never react to requests that are not for it's Unit ID. Can you paste your code into this ticket so I can see what you're doing?

NogginBops commented 6 years ago

This is what I do to create a slave.

slave = ModbusSlaveFactory.createSerialSlave(params);
img = factory.createProcessImageImplementation();
createRegisters(img); // Allocates registers
slave.addProcessImage(slaveID, img);
slave.open();

I looked in the source for the project and could not find anything that filters out messages received that are ment for other slaves.

steveohara commented 6 years ago

The filtering of messages is done in the listener. Take a look at line 166-167 of AbstractModbusListener. This infers that you must set a UnitID for your ProcessImage. Calling factory.createProcessImageImplementation() creates a SimpleProcessImage with a unit ID of 0.

A better way to do this would be to do the following;

slave = ModbusSlaveFactory.createSerialSlave(params);
img = new SimpleProcessImage(slaveID);
createRegisters(img); // Allocates registers
slave.addProcessImage(slaveID, img);
slave.open();

I think the j2mod code in this area needs simplification i.e. why does a process image need to know it's Unit ID if when it is added to the Slave it is allocated a Slave ID then? There is a refactor on the way that will remove the old ModbusCoupler and tidy up the handling off unit IDs for slaves but in the meantime, the above should work for you.

NogginBops commented 6 years ago

This worked thanks! The change of not having to specify the ID twice seems very logical to me. 😃

NogginBops commented 6 years ago

Never mind, this issue is not fixed...

And looking at the filtering code I'm a bit confused as it clearly does not filter any messages.

if (spi == null || (spi.getUnitID() != 0 && request.getUnitID() != spi.getUnitID())) {
    response = request.createExceptionResponse(Modbus.ILLEGAL_ADDRESS_EXCEPTION);
}

It clearly always sends an exception when there is no matching process image? Or is there something I am missing again?

steveohara commented 6 years ago

Can you try the latest snapshot and let me know how it goes please

NogginBops commented 6 years ago

I did a quick tests now to see if it worked by just plugging in the snapshot jar and it did not seem to work as correctly. As the master was now consistently getting read timeouts. I was not able to check if this was a configuration issue. But as this communication has worked with the fix I made so I doubt that is the problem. Sorry for the limited info, I can do a more thorough debugging of this on Wednesday.

NogginBops commented 6 years ago

Ok so it seems to work somewhat (It's not sending error responses). But now I'm experiencing another problem where reading or writing sometimes takes more than 1 second. And it seems really weird as I have set a read timeout of 300ms, I am running linux but from what I have gathered the read timeout should still work?

NogginBops commented 6 years ago

I was also wondering if the library implemented the inter-message wait time. For modbus RTU there should be a delay of minimum 3.5 characters between messages, and I don't seem to be able to find that in the code? Or is this missing?

steveohara commented 6 years ago

There is a delay you can set on the ModbusSerialTrsansaction object using setTransDelayMS - the default value is 0, it used to be 50msec.

You can't access this via the ModbusSerialMaster, you would have to construct requests using the mechanisms shown in the test ReadInputRegistersTest

That's not ideal so perhaps an overloaded version of the constructor to ModbusSerialMaster where you can specify this value?

NogginBops commented 6 years ago

I think that this should be standard for all RTU communication as it is specified in the specification. The time is computable from the baudrate.

steveohara commented 6 years ago

I've taken a good look at the specification and implemented timeouts as follows;

Master The transmission delay is now automatically calculated on the basis of the baud rate and the elapsed time since the last completed transaction. It calculates a delay based on 4 characters but uses a minimum of 2ms. You can also specify a fixed transmission delay on the constructor of the ModbusSerialMaster . This will override the auto-calculated value.

Slave The slave imposes a delay between receiving a request and sending a response using the baud rate, with a minimum value of 2ms.

These changes will be in tonight's snapshot.