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

Remove redundant check and guard debug output #438

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

Conversation

mhei
Copy link
Contributor

@mhei mhei commented Jun 19, 2018

I found two minor issues:

  • we can remove a redundant check
  • print error message only when debug output is enabled to be consistent with other functions

Edit: updated description after force-push (second issue was meanwhile fixed, so I dropped the commit)

Copy link
Contributor

@pboettch pboettch left a comment

Choose a reason for hiding this comment

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

FWIW, LGTM.

@mhei mhei force-pushed the remove-redundant-max-reg-check branch from d89f16e to 0c4b419 Compare December 4, 2022 14:18
@cla-bot cla-bot bot added the cla-signed label Dec 4, 2022
@mhei
Copy link
Contributor Author

mhei commented Dec 4, 2022

Stumbled over this old PR; issues are still present, so I've rebased and updated it.

@mhei
Copy link
Contributor Author

mhei commented Mar 19, 2023

Any reason to not merge this?

We have the same check already in exported functions modbus_read_registers
and modbus_read_input_registers which both calls out to the internal
static function read_registers.

So we can safely remove this redundant check in read_registers.

Signed-off-by: Michael Heimpold <[email protected]>
@mhei mhei force-pushed the remove-redundant-max-reg-check branch from 0c4b419 to 7b1409c Compare October 31, 2024 05:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants