Fix merge of spine data types when messages arrive with unset identifiers #52
+358
−30
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.
When a device sends e.g. a measurementListData response containing incomplete identifiers, and then sends a second measurementListData response containing complete identifiers the data from the second response is not correctly merged with the first response and queries to measurements by ID will always return the value of the first incomplete response and will never update to later values.
Our current approach implemented in this PR is to reject (negative acknowledgement) spine updates containing incomplete identifiers because to my knowledge devices really shouldn't be sending spine messages with incomplete identifiers (and to preclude needing to write a general merge function for generic spine data with potentially incomplete identifiers).
Commit bd9e7c8 adds a (initially failing) test for this scenario to help in reproduction and I've attached trace logs of this scenario happening on a real system below:
The identifiers in this case being the measurementId and the valueType, the first message only contains the measurementId while the second message contains both measerumentId and valueType. In the merge function the checked identifier for the first message is e.g. "4" while the checked identifier for the second message would be "4|value".
End result of this is that functions (such as in the eebus-go usecases) querying data receive multiple values for the same measurementId and have no way of identifying which of those should be correct.
This PR is in Draft state because I still wanted to test with the device producing this data how it responds to receiving a negative acknowledgement on the initial write with incomplete identifiers, and in case I might be missing something.