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

Fix custom message parsing in RTU framer #1716

Merged
merged 1 commit into from
Jul 31, 2023

Conversation

wlcrs
Copy link
Contributor

@wlcrs wlcrs commented Jul 31, 2023

As the regression reported in #1695 still isn't resolved in pyModbus 3.4.1, I've been tinkering with the code to try to reproduce the issue.

I noticed that ModbusRtuFramer caches the function codes registered in the decoder lookup table. As the framer is already initialized before a client can call the register() function to register custom Modbus message types, this prevents those custom Modbus messages from being processed correctly.

This PR uses .keys() instead of set() which prevents function_codes from becoming outdated if a custom message class is registered.

Open question: if there a valid case for the decoder being None? Otherwise we can drop the fallback to {}?

@janiversen
Copy link
Collaborator

May I politely correct you, the issue raised in #1695 is solved ! seems you are having another issue in addition to #1695.

Copy link
Collaborator

@janiversen janiversen left a comment

Choose a reason for hiding this comment

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

LGTM.

@janiversen janiversen merged commit 1b19039 into pymodbus-dev:dev Jul 31, 2023
@wlcrs wlcrs deleted the fix/rtu_framer_custom_response branch July 31, 2023 19:18
@janiversen
Copy link
Collaborator

When you call register() it should update the local copy, but your solution is more robust.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants