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.

Sign up for GitHub

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

Jump to bottom

Add Timestamped Nodes support to Send Operation at Server side. #1218

Closed
wants to merge 20 commits into from
Closed

Add Timestamped Nodes support to Send Operation at Server side. #1218

wants to merge 20 commits into from

Conversation

sbernard31
Copy link
Contributor

This PR includes #1217 with some fixes/cleaning.

This aims to implement #1192 at server side.

@sbernard31 sbernard31 mentioned this pull request Mar 10, 2022
Closed
@sbernard31
Copy link
Contributor Author

(I stopped for today but maybe tomorrow I will look at this a bit more)

@Michal-Wadowski
Copy link
Contributor

I checked your changes and I don't have any remarks. Thank you for the cleaning the code 😄

@sbernard31 sbernard31 mentioned this pull request Mar 14, 2022
Closed
@sbernard31
Copy link
Contributor Author

@Michal-Wadowski I added 2 more commits (not related to validation):

  • Fix LwM2mNodeDecoderTest.senml_multiple_timestamped_nodes
  • Make TimestampedLwM2mNodes really immutable

@sbernard31
Copy link
Contributor Author

@Michal-Wadowski, I do some modification to fix issue you raised and I also add some path/node consistency check.

From my point of view we are good to integrate this in master just let me know if you find more issue and/or if something is missing. :)

@Michal-Wadowski
Copy link
Contributor

Michal-Wadowski commented Mar 18, 2022 •
edited
Loading

@Michal-Wadowski, I do some modification to fix issue you raised and I also add some path/node consistency check.

From my point of view we are good to integrate this in master just let me know if you find more issue and/or if something is missing. :)

I checked the commits and all is OK for me except in the test should_raise_exception_id_path_does_not_match_node - there is unnecessary empty line at the end.

Good job, I'm very happy we worked out the solution together 😄

@sbernard31
Copy link
Contributor Author

the test should_raise_exception_id_path_does_not_match_node - there is unnecessary empty line at the end.

I fixed it.

Good job, I'm very happy we worked out the solution together smile

Thx to you. I very appreciate your contributions 🙏

sbernard31 pushed a commit that referenced this pull request Mar 18, 2022
@sbernard31
#1218: Add Timestamped nodes support to Send Operation at server side.
88d65ab
sbernard31 pushed a commit that referenced this pull request Mar 18, 2022
@sbernard31
#1218: Add Timestamped nodes support to Send Operation at server side.
633e3e2 
Signed-off-by: Michał Wadowski <Michal.Wadowski@orange.com>
Also-by: Simon Bernard <sbernard@sierrawireless.com>
@sbernard31
Copy link
Contributor Author

This is now integrated in master (commit 633e3e2)

@sbernard31 sbernard31 closed this Mar 18, 2022
@sbernard31 sbernard31 mentioned this pull request Mar 18, 2022
Closed
@sbernard31 sbernard31 mentioned this pull request Mar 28, 2022
Closed
@sbernard31 sbernard31 deleted the timestamped_nodes branch April 28, 2022 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Michal-Wadowski Michal-Wadowski Michal-Wadowski left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sbernard31 @Michal-Wadowski