Cloud-Automation / node-modbus

Modbus TCP Client/Server implementation for Node.JS
471 stars 175 forks source link

Create modbus-tcp-rtu-client.js #81

Closed GermanBluefox closed 7 years ago

GermanBluefox commented 7 years ago

Your pull request cover many topics. I see some basic code refactoring, a refactoring of the core server file, some additions to the modbus-serial-client and a new modbus-tcp-rtu file (I guess this is where you want to handle the tcp/rtu converter?), and last but not least the api changes input/discrete. I would like to have each of that in a separate commit so that we can talk about the more major changes in detail and do not get lost in too many topics. I can do that for you if you don't want to.

I don't know how to create one-file-pullrequest from existing PR. So I decided to create for every file separate pull request. Only one file should be taken in the 2.0.0

Cover your additions with tests. I don't see any tests for the serial client and tcp rtu files.

I could not see any test, that cover specific protocols, just core functionality. I would extend one existing, but there is nothing for that.

I plan to create tcp-rtu-server and serial server. But first my PRs should be merged, elsewise I will lost in changes.

The last PR from psorowka has broken the serial client. I have real users (ca 80) and they cover for me the test on real devices. They use all the combinations of client/server and serial-RTU/RTUviaTCP/TCP protocols.

Please appreciate my time. I have more than 2000 commits in year and this one PR was really a job. :)