steveohara / j2mod

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

Modbus Slave Multiple Port Support #23

Closed eli-mcgowan closed 8 years ago

eli-mcgowan commented 8 years ago

We need to be able to use multiple ports for our Modbus Slave project. From what I can see, this isn't possible because the ModbusCoupler is a singleton with only a single port assignment possible.

I am happy to put together a pull request with support for multiple ports, but I would appreciate your feedback on whether or not you are interested in including it in your library, and if so, what your preferred implementation would be. The two approaches that I see are

  1. change ModbusCoupler from a singleton to an instantiated class. This would break backwards compatibility.
  2. change ModbusCoupler to have a public constructor but continue to maintain support for the singleton ModbusCoupler. This would maintain backwards compatibility, but might look a little messy.

Any other suggested implementations are welcome too. Thanks!

steveohara commented 8 years ago

Hi, I'm very happy to take submissions, particularly those that improve the library.

To be absolutely honest, the Slave implementation in j2mod is extremely poor in my opinion. Apart from fixing the obvious coding issues, I haven't changed it at all since the migration.

If we are going to change it, then let's do it without compromise and not worry to much about backwards compatibility at this stage. If an implementation can be designed that allows the shonky ModbusCoupler to live on, then fine but quite frankly, it hurts my eyes when I look at that (even the name offends me).

I would propose a factory pattern ModbusSlaveManager (singleton or static, I don't have a preference) that you call to create ModbusSlave objects given a ProcessDefinition. The factory would manage the issuing and returning of the slaves, static methods that can be called to return a running Slave instance given a Transaction ID and Unit ID etc. When a ModbusSlave is closed, it removes itself from the running list in ModbusSlaveManager.

If we did it this way, I can see how we can change ModbusCoupler to carry on for backwards compatibility, simply a wrapper around a ModbusSlave.

This actually doesn't look like too much work so if you want to do it great, if you prefer me to put it together I can take a look at the weekend. What do you think?

eli-mcgowan commented 8 years ago

From the sound of it, you have a better grasp of the problem than I do, so it would be great if you could have a look at it. If something comes up and you aren't able to get around to it, I will give it a go next week.

Thanks!

steveohara commented 8 years ago

An update: I've implemented the changes and the regression tests are all working OK which means that the ModbusCoupler mechanism hasn't been adversely affected. I need to create some more unit tests and will hopefully publish a snapshot version for your testing on Wednesday.

As per my description above, there is a factory (ModbusSlaveFactory) that allows you to create ModbusSlave objects - each object listens on a port and can have one or more ProcessImage objects associated with it. This allows you to share images across ports if you want to.

Here is an example;

ProcessImage image = makeMyProcessImage(); ProcessImage image1 = makeMyProcessImageAgain();

ModbusSlave slave502 = ModbusSlaveFactory.createTCPSlave(502, 5); slave502.addProcessImage(image); slave502.addProcessImage(image1); slave502.open(); ModbusSlave slave503 = ModbusSlaveFactory.createTCPSlave(503, 5); slave503.addProcessImage(image); slave503.open(); ..... slave502.close(); slave503.close(); // or // ModbusSlaveFactory.close(); // Closes all slaves

steveohara commented 8 years ago

Right, this change is now complete. I would appreciate it if you could download the snapshot and give it a bit of a workout and confirm you are happy with it

https://oss.sonatype.org/content/repositories/snapshots/com/ghgande/j2mod/2.2.0-SNAPSHOT/j2mod-2.2.0-20160704.202616-1-javadoc.jar

There is a Wiki page that shows how to use the new interfaces

j123b567 commented 8 years ago

It would be better to also allow listening on the same TCP and UDP port. So something like slaves.put("udp" + port, slave); slaves.put("tcp" + port, slave);

eli-mcgowan commented 8 years ago

The multiple port support is working incredibly well. Tested with 14 simulated devices (ClearSCADA Modbus), using 2 ports and reusing some Modbus addresses on both ports.

Awesome improvement - Thanks!

steveohara commented 8 years ago

Excellent news. I've published a production version with this enhancement and a few other fixes from other people as v2.2.0