SMHE-1609: Improve Mx382AlarmDecoder handling #1041
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Andre Bolder <[email protected]>
stefanermens
requested changes
Jul 21, 2023
|
|
||
| // Datetime is not used, so can be skipped as well | ||
| this.skipDateTime(inputStream); | ||
| // Datetime is read and copied |
There was a problem hiding this comment.
The datetime bytes are read but not used
There was a problem hiding this comment.
Modified the comment that the datetime is skipped.
| // The next byte of the datetime is the length byte | ||
| final byte lengthOfData = this.readByte(inputStream); | ||
| this.readBytes(inputStream, lengthOfData); | ||
| } |
There was a problem hiding this comment.
Why are the dateTime bytes not appended?
There was a problem hiding this comment.
The datetime is not used in this message and for other messages the dates are also skipped.
| private void addAlarm() { | ||
| this.builder.withTriggerType(PUSH_ALARM_TRIGGER); | ||
| final Set<AlarmTypeDto> alarmSet = new HashSet<>(); | ||
| alarmSet.add(AlarmTypeDto.LAST_GASP); | ||
| this.builder.addAlarms(alarmSet); | ||
| } | ||
|
|
||
| private void checkByte(final InputStream inputStream, final byte expectedByte, final String name) | ||
| private void handleByte(final InputStream inputStream, final byte expectedByte, final String name) |
There was a problem hiding this comment.
Why is this renamed to handleByte? It seems to do the same as checkBytes, but then only for 1 byte.
There was a problem hiding this comment.
renamed back to checkBytes
| @@ -39,16 +45,52 @@ void testDecode() throws UnrecognizedMessageDataException { | |||
| assertThat(dlmsPushNotification.getEquipmentIdentifier()).isEqualTo(this.equipmentIdentifier); | |||
| } | |||
|
|
|||
| /* | |||
| * Decode with an alarm message that contains a date time stamp, this has an alternative length withing the WDPU header (2a on index 7, 8th byte). | |||
There was a problem hiding this comment.
within instead of withing
added 2 commits
July 21, 2023 11:12
… UnrecognizedMessageDataException Signed-off-by: Andre Bolder <[email protected]>
Signed-off-by: Andre Bolder <[email protected]>
stefanermens
approved these changes
Jul 21, 2023
harrymiddelburgetall
approved these changes
Jul 21, 2023
Signed-off-by: Andre Bolder <[email protected]>
|
Kudos, SonarCloud Quality Gate passed! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Accept Mx382AlarmDecoder message with date and time within the message so alarm will be forwarded.