Skip to content

[EDGE] Handle UNDEFINED_VALUE as null in ModbusRecordChannel#3601

Open
Sn0w3y wants to merge 2 commits intoOpenEMS:developfrom
Sn0w3y:null-modbus-fix
Open

[EDGE] Handle UNDEFINED_VALUE as null in ModbusRecordChannel#3601
Sn0w3y wants to merge 2 commits intoOpenEMS:developfrom
Sn0w3y:null-modbus-fix

Conversation

@Sn0w3y
Copy link
Collaborator

@Sn0w3y Sn0w3y commented Mar 2, 2026

This commit updates the ModbusRecordChannel to interpret UNDEFINED_VALUE as null for numeric types (FLOAT64, FLOAT32, UINT16, UINT32, UINT64). This change ensures that when a Modbus record contains a value that is designated as undefined or not applicable, it is correctly handled as a null value in the system, rather than being processed as a valid numeric value. This is crucial for avoiding incorrect data interpretation and ensuring data integrity in applications that rely on Modbus communication.

This commit updates the ModbusRecordChannel to interpret UNDEFINED_VALUE as null for numeric types (FLOAT64, FLOAT32, UINT16, UINT32, UINT64). This change ensures that when a Modbus record contains a value that is designated as undefined or not applicable, it is correctly handled as a null value in the system, rather than being processed as a valid numeric value. This is crucial for avoiding incorrect data interpretation and ensuring data integrity in applications that rely on Modbus communication.
@codecov
Copy link

codecov bot commented Mar 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

Additional details and impacted files
@@              Coverage Diff              @@
##             develop    #3601      +/-   ##
=============================================
+ Coverage      58.50%   58.53%   +0.03%     
  Complexity       104      104              
=============================================
  Files           3095     3095              
  Lines         134204   134215      +11     
  Branches        9872     9871       -1     
=============================================
+ Hits           78501    78547      +46     
+ Misses         52778    52749      -29     
+ Partials        2925     2919       -6     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@sfeilmeier sfeilmeier left a comment

Choose a reason for hiding this comment

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

Thanks - did you try this? We'll definitely need proper JUnit tests here - maybe by moving the switch-case to a testable static method.

Probably it would be also better to compare the byte-array with UNDEFINED before converting the value to an Object. That way we could avoid the type-casting.

This commit refactors the value parsing mechanism in ModbusRecordChannel to improve code readability and maintainability. Previously, the code used a ByteBuffer to convert the writeValueBuffer directly within the writeValue() method, which included manual buffer management and type-based value extraction. Now, the parsing logic is extracted into a separate, static parseValue() method, which simplifies the writeValue() method by directly converting the writeValueBuffer to a byte array and then parsing it according to the ModbusType. This change not only makes the code cleaner by separating concerns but also adds support for parsing STRING16 types, which was marked as a TODO in the previous implementation. Additionally, the commit includes comprehensive unit tests for the new parsing method, ensuring that all ModbusType values are correctly handled, including the special case of UNDEFINED values.
@Sn0w3y Sn0w3y requested a review from sfeilmeier March 3, 2026 08:11
@Sn0w3y
Copy link
Collaborator Author

Sn0w3y commented Mar 3, 2026

Thanks - did you try this? We'll definitely need proper JUnit tests here - maybe by moving the switch-case to a testable static method.

I in fact did not try this - maybe we could make a call out in community or use the FEMS Customer in community as tester if possible on your side? :)
The UNIT Tests should cover the new Code now pretty well

@sfeilmeier
Copy link
Contributor

I in fact did not try this - maybe we could make a call out in community or use the FEMS Customer in community as tester if possible on your side? :) The UNIT Tests should cover the new Code now pretty well

No sorry, that would be a lot of effort. Most of this code should be quite easily testable with Simulated ESS directly in the IDE.

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