Skip to content

Fix SAT errors#2027

Open
kaikreuzer wants to merge 9 commits into
mainfrom
sat
Open

Fix SAT errors#2027
kaikreuzer wants to merge 9 commits into
mainfrom
sat

Conversation

@kaikreuzer
Copy link
Copy Markdown
Member

No description provided.

Signed-off-by: Kai Kreuzer <kai@openhab.org>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR targets Static Analysis Tool (SAT) findings across the Z-Wave binding by cleaning up test code (logging, imports) and applying small production code refactors (collection emptiness checks, logging format fixes, exception type refinement).

Changes:

  • Replace printStackTrace() / System.out.println() usage in tests with SLF4J logging and narrow wildcard static imports.
  • Refactor some collection empty checks to isEmpty() and adjust several logging statements to pass Throwable correctly.
  • Update ZWaveTransactionManager transaction comparisons and refine an exception type in ZWaveSecurityCommandClass.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 25 comments.

Show a summary per file
File Description
src/test/java/org/openhab/binding/zwave/internal/protocol/ZWaveTransactionManagerTestSync.java Replace stack traces with SLF4J logging; narrow static imports.
src/test/java/org/openhab/binding/zwave/internal/protocol/ZWaveTransactionManagerTestBasic.java Add SLF4J logging for interrupted sleep; import changes.
src/test/java/org/openhab/binding/zwave/internal/protocol/serialmessage/ZWaveCommandProcessorTest.java Replace stack trace printing with logging.
src/test/java/org/openhab/binding/zwave/internal/protocol/serialmessage/IdentifyNodeMessageClassTest.java Replace stack trace printing with logging.
src/test/java/org/openhab/binding/zwave/internal/protocol/serialmessage/ApplicationUpdateMessageClassTest.java Narrow assertions import; replace stack trace printing with logging.
src/test/java/org/openhab/binding/zwave/internal/protocol/serialmessage/AddNodeMessageClassTest.java Remove System.out.println; add logging in exception path; adjust imports.
src/test/java/org/openhab/binding/zwave/internal/protocol/commandclass/ZWaveCommandClassPayloadTest.java Narrow assertions import; replace stack trace printing with logging.
src/test/java/org/openhab/binding/zwave/internal/converter/ZWaveThermostatSetpointConverterTest.java Replace reflection exception stack traces with logging; add SLF4J imports.
src/test/java/org/openhab/binding/zwave/handler/ZWaveThingHandlerTest.java Replace reflection exception stack traces with logging; add SLF4J imports.
src/main/java/org/openhab/binding/zwave/internal/protocol/ZWaveTransactionManager.java Replace some == comparisons with equals(); use isEmpty() for collection check.
src/main/java/org/openhab/binding/zwave/internal/protocol/initialization/ZWaveNodeInitStageAdvancer.java Replace size check with isEmpty().
src/main/java/org/openhab/binding/zwave/internal/protocol/commandclass/ZWaveSecurityCommandClass.java Throw IllegalArgumentException on parse error; adjust error logging format.
src/main/java/org/openhab/binding/zwave/internal/protocol/commandclass/ZWaveMultiInstanceCommandClass.java Replace size check with isEmpty().
src/main/java/org/openhab/binding/zwave/internal/protocol/commandclass/ZWaveMeterTblMonitorCommandClass.java Fix SLF4J logging to include exception properly.
src/main/java/org/openhab/binding/zwave/internal/protocol/commandclass/ZWaveAssociationGroupInfoCommandClass.java Replace size check with isEmpty().

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Kai Kreuzer <kai@openhab.org>
kaikreuzer and others added 4 commits May 11, 2026 14:25
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Kai Kreuzer <kai@openhab.org>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Kai Kreuzer <kai@openhab.org>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Kai Kreuzer <kai@openhab.org>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Kai Kreuzer <kai@openhab.org>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Kai Kreuzer <kai@openhab.org>
@apella12
Copy link
Copy Markdown
Contributor

apella12 commented May 12, 2026

Don't know the preference of maintainers, but on the firmware PR (unmerged) I replaced the e.printStackTrace(); with fail(e);
from org.junit.jupiter.api.Assertions.*; (already imported). Since the tests just run during compile, a fail(e) seemed simplier than logging.

        } catch (NoSuchFieldException | SecurityException e) {
            fail(e);
        } catch (IllegalArgumentException e) {
            fail(e);
        } catch (IllegalAccessException e) {
            fail(e);

Just a thought

@kaikreuzer
Copy link
Copy Markdown
Member Author

@apella12 Fair enough as well. I opted for merely switching out the printing method and thus not changing anything about the test's flow. With fail() it might actually fail a test although it didn't do it before (because the exception might not have indicated a failure).

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.

4 participants