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.

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

Add block-wise options #335

Merged
merged 18 commits into from
Mar 27, 2025
Merged

Conversation

zalenskivolt
Copy link
Contributor

@zalenskivolt zalenskivolt commented Mar 22, 2025

see IANA CoAP Option Numbers

# Option RFC
19 Q-Block1 [RFC9177 Block-Wise robust]
23 Block2 [RFC7959 Block-Wise][RFC8323 TLS+][RFC8613 OSCORE]
27 Block1 [RFC7959 Block-Wise][RFC8323 TLS+][RFC8613 OSCORE]
28 Size2 [RFC7959 Block-Wise][RFC8613 OSCORE]
31 Q-Block2 [RFC9177 Block-Wise robust]
(60 Size1) [RFC7252 CoAP][RFC8613 OSCORE]

(Size1 was already implemented, some tests added)

TLS+ = TCP, TLS, and WebSockets

@twyatt

This comment was marked as outdated.

@twyatt

This comment was marked as outdated.

@twyatt twyatt added the minor Changes that should bump the MINOR version number label Mar 24, 2025
@zalenskivolt zalenskivolt force-pushed the block-wise-options branch 2 times, most recently from 455e5cc to 452c508 Compare March 24, 2025 22:01
Copy link

codecov bot commented Mar 24, 2025

Codecov Report

Attention: Patch coverage is 73.17073% with 22 lines in your changes missing coverage. Please review.

Project coverage is 63.95%. Comparing base (74fdfac) to head (0cdb249).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
koap-core/src/commonMain/kotlin/Block.kt 47.61% 8 Missing and 3 partials ⚠️
koap-core/src/commonMain/kotlin/Message.kt 78.43% 5 Missing and 6 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #335      +/-   ##
==========================================
+ Coverage   61.72%   63.95%   +2.22%     
==========================================
  Files           8        9       +1     
  Lines         695      774      +79     
  Branches      156      172      +16     
==========================================
+ Hits          429      495      +66     
- Misses        215      220       +5     
- Partials       51       59       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 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.

@zalenski
Copy link

when you have a chance, can you rebase/fix conflicts? TY!

@twyatt rebased & fixed

@twyatt

This comment was marked as outdated.

@twyatt
Copy link
Member

twyatt commented Mar 26, 2025

@zalenskivolt I pushed a couple commits to restructure things a little.

I changed the size to an enum to enforce only specific sizes, but I'm not sure if it is the best approach (the enum naming is awkward) but it does provide better compile-time safety. 🤷

Let me know what you think.

@twyatt twyatt requested a review from zalenski March 26, 2025 05:53
Copy link
Contributor

@sdonn3 sdonn3 left a comment

Choose a reason for hiding this comment

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

Very clean, tested and well documented, I love it!

@twyatt twyatt enabled auto-merge (squash) March 27, 2025 20:18
@twyatt twyatt merged commit 3e15cd8 into JuulLabs:main Mar 27, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor Changes that should bump the MINOR version number
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants