search

Cloud-Automation / node-modbus

Modbus TCP Client/Server implementation for Node.JS
465 stars 174 forks source link

RTU Server support #204

Closed martijnthe closed 6 years ago

martijnthe commented 6 years ago

Hi @stefanpoeter ,

I added RTU server support in this PR. I tried to re-use the existing code as much as possible by making the various TCP classes generic/common to both TCP and RTU.

The tests I added are basically a copy of the ones in tcp-server.test.js, which the exception that CRCs are of course part of the payloads. I also added a test case there for a bad CRC / corrupt data. I'm not sure if there's much value in "duplicating" more of the test cases. It looks like tcp-server.test.js already covers all already.

I've only tested using the unit tests. I'll be testing with actual hardware today. Fingers crossed ;)

"Fixes" #201

stefanpoeter commented 6 years ago

Thanks @martijnthe for your contribution. You are right, you do not need to cover all fcs in the rtu server tests.

I'll review your code on the weekend :-)

martijnthe commented 6 years ago

Thanks.

I just noticed the 16-bit CRCs are written in little endian order, while the rest of the protocol seems to be using big endian order. This was already the case before I started touching the code, but it seems incorrect to me.

I'll been testing with a real device soon, so I'll know soon what it's supposed to be...

martijnthe commented 6 years ago

Oh, just reading the spec -- oddly, the CRC does seem to be LE (!?):

The CRC field is appended to the message as the last field in the message. When this is done, the low–order byte of the field is appended first, followed by the high–order byte. The CRC high–order byte is the last byte to be sent in the message.

stefanpoeter commented 6 years ago

Yes the crc swap bytes! Did you test your code with a real device?

martijnthe commented 6 years ago

@stefanpoeter yes, this branch is working for me with a real device.

stefanpoeter commented 6 years ago

@martijnthe I've reviewed your code, thanks a lot.

Since this introduces RTU Server this should be version 3.1.0, can you add this to the package.json. I'll rename the branch after the merge, will then test it a bit and then publish it after I've setup a new virtual server for long-term testing.

martijnthe commented 6 years ago

👍

stefanpoeter commented 6 years ago

See branch v3.1.0-dev