steveohara / j2mod

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

Fixed slave responding to wrong request #73

Closed NogginBops closed 6 years ago

NogginBops commented 6 years ago

This makes the slave implementation not answer modbus messages not intended for the slave. Fixes #62

steveohara commented 6 years ago

Thanks for the fix - sorry to be a pain, but I'm getting stricter with taking code commits without unit tests. Can you check to see if there are any unit tests that cover this scenario and if not, add one?

NogginBops commented 6 years ago

I'll see if I can do something about the tests. :smiley:

NogginBops commented 6 years ago

There looks like there is a test in TestModbusSerialRTUMasterRead.java - testBadUnitIdRequest()

@Test
public void testBadUnitIdRequest() {
    try {
        master.readCoils(UNIT_ID + 10, 0, 1).getBit(0);
        fail("Failed check for invalid Unit ID");
    }
    catch (Exception e) {
        logger.info("Got expected error response (testBadUnitIdRequest) - {}", e.getMessage());
    }
}

But the question here is how the test passed before. I have no way to actually run the serial tests on actual hardware, do you have the ability to do this?

steveohara commented 6 years ago

I use com0com on a Windows machine to test the serial implementation

steveohara commented 6 years ago

Ok, I understand the problem now. However, your fix is not correct because an error response should be sent back for TCP/UDP slaves. The response should not be sent back for Serial slaves only.

I'm working on a fix now.