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

[modbus] reduce log level when modbus slave returns DEVICE_BUSY exception #3847

Merged
merged 2 commits into from
Apr 28, 2024

Conversation

nagisa
Copy link
Contributor

@nagisa nagisa commented Oct 16, 2023

This exception is meant to indicate that the request should be retried shortly, essentially, and at least some of the devices I own seem to be busy bees. Thus my logs receive significant spam of this warning.

Since the exception is transient and retrying it is the expected course of action, I think it makes sense to reduce the log level here slightly and only output an error when the retries get exhausted.

…tion

This exception is meant to indicate that the request should be retried
shortly, essentially, and at least some of the devices I own seem to be
busy bees. Thus my logs receive significant spam of this warning.

Since the exception is transient and retrying it is the expected course
of action, I think it makes sense to reduce the log level here slightly
and only output an error when the retries get exhausted.

Signed-off-by: Simonas Kazlauskas <git@kazlauskas.me>
@nagisa nagisa requested a review from a team as a code owner October 16, 2023 17:15
@@ -662,7 +668,11 @@ private <R, C extends ModbusResultCallback, F extends ModbusFailureCallback<R>,
} catch (ModbusSlaveException e) {
lastError.set(new ModbusSlaveErrorResponseExceptionImpl(e));
// Slave returned explicit error response, no reason to re-establish new connection
if (willRetry) {
if (willRetry && e.getType() == MODBUS_EXCEPTION_SLAVE_DEVICE_BUSY) {
logger.info(
Copy link
Member

Choose a reason for hiding this comment

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

When re-trying is an expected operation, you can reduce that to DEBUG.

@lolodomo
Copy link
Contributor

@nagisa : did you see the review comment?

…/openhab/core/io/transport/modbus/internal/ModbusManagerImpl.java

Signed-off-by: Simonas Kazlauskas <github@kazlauskas.me>
@lolodomo
Copy link
Contributor

@J-N-K : you can probably merge this PR now.

@wborn wborn requested a review from J-N-K December 19, 2023 20:38
@wborn wborn requested review from J-N-K and removed request for J-N-K January 2, 2024 09:41
@wborn wborn added bug An unexpected problem or unintended behavior of the Core and removed awaiting feedback labels Jan 4, 2024
Copy link
Member

@J-N-K J-N-K left a comment

Choose a reason for hiding this comment

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

Thanks! And sorry for letting you wait so long. I didn't see my comments were addressed.

@J-N-K J-N-K merged commit 81f2bd9 into openhab:main Apr 28, 2024
3 checks passed
@J-N-K J-N-K added this to the 4.2 milestone Apr 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An unexpected problem or unintended behavior of the Core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants