-
-
Notifications
You must be signed in to change notification settings - Fork 156
feat: add _lastMessageId tracking to ObserveReadStream #396
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.
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
Conversation
Pull Request Test Coverage Report for Build 16913726014Details
💛 - Coveralls |
Pull Request Test Coverage Report for Build 19016209961Details
💛 - Coveralls |
|
I restarted the tests .. lets see if they pass now |
|
grmppf seems we need to find out what the tests are that flaky currently :-( @JKRhb I will be on vacation the next two weeks so no clue if I find time before mid september ... I can do reviews from my mobile, but likely no development or testing that out. But seems like the same issue already appeared with the Deps PR ... |
Summary of CI Test FixesFixed 3 environment-specific test failures - no library code changes needed: 1. Multicast tests (6 failures) - Added CI detection to skip when multicast networking is unavailable in CI environments ( 2. Content-Format test - Fixed race condition by registering event listeners before sending the test request instead of after. 3. Retry test on Windows - Extended timeout from 45s to 50s and made assertion platform-aware: accepts 4-5 retry messages on Windows (vs exactly 5 on Linux/macOS) to account for fake timer precision differences. All tests now pass reliably in CI across Ubuntu, Windows, and macOS runners. Test coverage is maintained on all platforms. |
|
Can you please review? @Apollon77 @JKRhb |
JKRhb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the changes in general look good – thank you for doing the work of fixing the tests! :) See one comment below.
JKRhb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Summary
This MR adds message ID tracking capability to the
ObserveReadStreamclass, enhancing the observability of CoAP observe operations.Changes Made
Code Changes
_lastMessageIdproperty toObserveReadStreamclass inlib/observe_read_stream.tsnumber | undefinedto handle cases where no messages have been processed yetundefinedin constructorappend()method when processing observe packetsTest Coverage
test/agent.tswithin the existing "observe problems" test suite_lastMessageIdTechnical Details
The
_lastMessageIdproperty is updated in theappend()method at line 55:This happens when a packet passes the filtering criteria and is pushed to the stream, ensuring the property always reflects the most recently processed message ID.
Benefits
_lastIdand_lastTimepropertiesTesting
Files Changed
lib/observe_read_stream.ts- Added_lastMessageIdproperty and logictest/agent.ts- Added test coverage for new functionalityBreaking Changes
None. This is a purely additive change that maintains backward compatibility.