Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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 <mhei@heimpold.de>
@mhei mhei force-pushed the remove-redundant-max-reg-check branch from 0c4b419 to 7b1409c Compare October 31, 2024 05:56
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants