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

Cleanup ReadWriteMultipleRegistersResponse and testing #2261

Merged
merged 3 commits into from
Jul 24, 2024
Merged

Conversation

alexrudd2
Copy link
Collaborator

@alexrudd2 alexrudd2 commented Jul 23, 2024

There is no reason ReadWriteMultipleRegistersResponse can't inherit from ReadHoldingRegistersResponse. The sole differences are:
(1) the function code (OK)
(2) decode() not initializing self.registers

def decode(self, data):
"""Decode the register response packet.
:param data: The response to decode
"""
bytecount = int(data[0])
for i in range(1, bytecount, 2):
self.registers.append(struct.unpack(">H", data[i : i + 2])[0])

This is a bug - if for some dumb reason decode() is called twice, it will keep appending registers to self.registers. As far as I can tell, this bug (and corresponding incorrect test code) has been there since b02f5fe (13 years!)

Changing this also makes it apparent that the test code can be readily simplified.

@alexrudd2 alexrudd2 changed the title Cleanup ReadHoldingRegistersResponse and testing Cleanup ReadWriteMultipleRegistersResponse and testing Jul 23, 2024
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.

Wow, nice cleanup!

@janiversen janiversen merged commit 371e168 into dev Jul 24, 2024
1 check passed
@janiversen janiversen deleted the rw-inherit-r branch July 24, 2024 06:27
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