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

Check byte count in DetectFrame method. #75

Merged
merged 7 commits into from
Jul 28, 2022
Merged

Check byte count in DetectFrame method. #75

merged 7 commits into from
Jul 28, 2022

Conversation

iberisoft
Copy link
Contributor

@iberisoft iberisoft commented Jul 24, 2022

Hello @Apollo3zehn,

I have found an issue with the current logic of how the Modbus RTU frame is detected. This issue occurs under seldom circumstances, let me share an example.

We have a temperature sensor with unit identifier 1 providing data thru input register 0. The corresponding Modbus request frame is 01 04 00 00 00 01 31 CA. Response frames depend on the actual temperature getting as Celsius degrees multiplied by 100. However, there is a special value that we must focus on: 7.69 degrees. The Modbus response frame for value 769 is 01 04 02 03 01 78 00.

The DetectFrame method is called from the response pump. Our sensor returns a frame as entirely (7 bytes) as divided into segments (e.g. 6+1 bytes). The described issue occurs when the DetectFrame method is called for the response frame mentioned in the previous paragraph without the last byte (6-byte incomplete segment).

Theoretically, such trimmed frames should be identified in DetectFrame due to relying on CRC. Unfortunately, the CRC-based check is not enough for this particular case. Look at the discussed response frame trimmed to 01 04 02 03 01 78. The frame-detection code treats this frame as successfully detected because sequence 01 04 02 03 generates CRC 0x7801. Obviously, 01 04 02 03 01 78 is incomplete by the standard because it indicates 2 bytes of data in the third byte but it has only one data byte.

So I have added a couple of lines into the method to additionally check the byte count field declared by the Modbus RTU specification.

@iberisoft iberisoft marked this pull request as ready for review July 24, 2022 21:44
@Apollo3zehn
Copy link
Owner

Apollo3zehn commented Jul 26, 2022

Good catch & thanks for the PR! Your solution makes sense. However, could you please extend your frame length check to make sure it is not a Modbus error frame? I think something like if (span[1] < 0x80 && span.Length < span[2] + 5) would be enough.

Here is the layout of both frame types:

   /* Correct response frame (min. 6 bytes)
    * 00 Unit Identifier
    * 01 Function Code
    * 02 Byte count
    * 03 Minimum of 1 byte
    * 04 CRC Byte 1
    * 05 CRC Byte 2
    */

   /* Error response frame (5 bytes)
    * 00 Unit Identifier
    * 01 Function Code + 0x80
    * 02 Exception Code
    * 03 CRC Byte 1
    * 04 CRC Byte 2
    */

@iberisoft
Copy link
Contributor Author

Thank you for reminding the error response case!

@iberisoft
Copy link
Contributor Author

iberisoft commented Jul 27, 2022

@Apollo3zehn, after your approval in branch master, please copy the discussed fix to branch v3-backport and build version 3.2.2. We do not use version 4.x-preview, only the stable version.

@Apollo3zehn Apollo3zehn changed the base branch from master to dev July 28, 2022 07:13
Copy link
Owner

@Apollo3zehn Apollo3zehn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@Apollo3zehn Apollo3zehn merged commit e04ffcf into Apollo3zehn:dev Jul 28, 2022
Apollo3zehn added a commit that referenced this pull request Jul 28, 2022
Check byte count in DetectFrame method.
@Apollo3zehn
Copy link
Owner

@iberisoft The package is released now, also as v3.2.2.

@jmsqlr
Copy link

jmsqlr commented Nov 5, 2022

if (span[1] < 0x80 && span.Length < span[2] + 5)
Span[2] is the low byte for Starting address on requests.
ModbusServer fails due to this change on client requests with starting address low byte > span[2]+4.

Best regards.

@Apollo3zehn
Copy link
Owner

You are right

grafik

My fault, I will change that next week.

Apollo3zehn pushed a commit that referenced this pull request Nov 14, 2022
@Apollo3zehn
Copy link
Owner

@jmsqlr, it should be solved now. When I started developing FluentModbus, I was on Windows and had no easy way to test the Modbus RTU part automatically. Now I am on Linux and the project is compiled using GitHub Actions. So now we can maybe link two virtual COM ports to run Modbus RTU unit tests to avoid issues like the one you described.

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

Successfully merging this pull request may close these issues.

3 participants