Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modbus-serial buffered mode throws exception instead of data #58

Closed
SuperJojo2001 opened this issue Feb 9, 2017 · 8 comments
Closed

Comments

@SuperJojo2001
Copy link
Contributor

Hello I am using Modbus-Serial along with "node-red-contrib-modbus" on an embedded intel platform. I must admit that the platform is that fast that it always return serial receive strings in chunks. This is why I am using buffered RTU mode. It works well except the following case:

I am sending a read multiple register command of 64 registers to a modbus RTU slave no with address 1.

Modbus serial sends out the hex string 010300000040443A which is absolutely correct.

My slave answers also correct with 133 bytes and in each of the 64 returned value it reports 0183hex = 387dec

010380018301830183018301830183018301830183018301830183018301830183018301830183018301830183018301830183018301830183018301830183018301830183018301830183018301830183018301830183018301830183018301830183018301830183018301830183018301830183018301830183018301830183018346e0

Now comes the point:

The first chunk received in the buffered function this._client.on('data', function onData(data) {...} are the first 9 bytes ...

010380018301830183

So just to set the values right during the first loop cycle as the code would do it:

expectedLength = 133;
bufferLength = 9;
maxoffset = 9 - 5 = 4;
unitid = 1
functioncode = 3

If you now walk through the for next loop you come to the point where i = 3 and then by mistake
unitId gets 01 and functioncode get 83 (cause it is part of my data) and the functions throws a
self._emitData(i, EXCEPTION_LENGTH);

This is a bug.

It is better to evaluate unitid and function by takeing it from the [0],[1] bytes of the chunk outisde the loop outside the loop

var unitId = self._buffer[0];
var functionCode = self._buffer[1];

Also I would check the buffer so long as the whole data stream with the calculated expected length is received and then I would evaluate the whole string.

@yaacov
Copy link
Owner

yaacov commented Feb 9, 2017

Great !
a. can you make a pull request ?
b. can you add this case to the tests ?

@SuperJojo2001
Copy link
Contributor Author

Hello Yaacov,

before I pull ... and excuse me that I am asking ... I want to understand the current code and what was the initial intention about it in order not destroy your "basic thinking" with my code.

In the buffer examining loop I see the code line
if (unitId !== self._id) continue;
So it loops until it finds the unitid as initial char in the load of the RX buffer ... am I correct that you intended to create a kind of synchronization of where a modbus receive telegram starts in the current load of the RX buffer? Please confirm.

Cause if answer is yes I do have doubts that this will work, cause in my life example you see that also modbus data itself can have the same combo set of unitid and function code. So with this method you will never detect a start of frame securely. Do you agree?

Also I recognized that you are checking for the length of an exception message ... you check for (0x80 | self.cmd) ... this is fine for me ... but don't you agree that the examination of just the unitid and function code | 0x80 you can never be 100% sure that is a real exception message without having also the checksum checked at this point?

@SuperJojo2001
Copy link
Contributor Author

Hello Yaacov,

i have another dumb question.

Let us suppose I have a "bad working" Modbus RTU slave online that responds instead of an exception or a valid frame with endless chars like e.g. "aaaaaaaa....." forever and forever ... or frames that are anything else than Modbus RTU frames ... in this case the reception "self._buffer" will grow and grow and finally would generate if you wait long enough a "out of memory" error. Do you agree?

So we need also a mechanism to "throw" away frames that are no modbus frames. Do you agree?

@yaacov
Copy link
Owner

yaacov commented Feb 9, 2017

hi,

a. Yes, we need to deal with modbus frames that start not at the beginning of buffer.
b. Out of memory is something important to fix, maybe in a different pull request ?
c. Make the best you can do, if the tests work, it will be ok, if it start break things that we miss in the test, we will fix it again.

p.s.
Their is always a trade-off between something that we can do and works good, and something that is perfect, but we do not have time to do ... :-)

@SuperJojo2001
Copy link
Contributor Author

Hi, I finished development but before I am pulling it I want to test it with the test environment

This is why I have extended the rtubufferedport.test.js file locally with my life example above.

But when I start it with "node rtubufferedport.test.js" it runs and exits without any feedback. How do I start this test correctly?

@yaacov
Copy link
Owner

yaacov commented Feb 10, 2017

run (from root of prject)
npm test

@yaacov
Copy link
Owner

yaacov commented Feb 10, 2017

p.s.
a. when you do a pull request, tests are run automatically.
b. other people can see what you are doing and comment about it.

@yaacov
Copy link
Owner

yaacov commented Feb 10, 2017

fixed by #60 and #59

@yaacov yaacov closed this as completed Feb 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants