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

- Added flag to receive packets with invalid CRC as well #301

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lmartorella
Copy link

Hi,
I'm coping right now with a brand new solar inverter that pretends to talk Modbus on RS485. However, if I invoke function 0x3 to read registers and an invalid range is specified (the addressing seems to be split in many different sub-ranges), instead of answering:

[0x01] [0x83] [0x02] [CRC] [CRC]

(where 0x1 is obviously the node Id), it always wrongly respond with:

[0x01] [0x90] [0x02] [0x00] [0x00]

with two errors: respond to the function 0x10 + error flag, rather than the 0x3, and it fails to present a valid CRC, the two bytes are always zero.
I'm not sure if this is a firmware issue that will be corrected in the future, or if it is part of a some non-standard code.

However, I was interested in receiving such strange errors and process them correctly. Currently, the library rejects the packet due to CRC error.

The idea is to be able to receive malformed packet (I can see there is already a partial support there). However, for compatibility, I suggest to add a new argument to the onRaw, in order to specify the willing to receive also CRC-failed packets.

In addition, I added a validFrame boolean flag in the frame_arg_t argument, that is not set in case of CRC errors or in the previously covered cases of wrong node id.

I separated the onRaw of the base class, used by the TCP implementation, since CRC is only on the RTU side.

Please let me know what you think about it.
Thanks! L.

@emelianov emelianov added this to the 4.1.2 milestone Oct 15, 2023
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.

2 participants