shamblett / coap

A Coap package for dart
Other
16 stars 13 forks source link

feat!: rework URI handling for CoAP requests and responses #169

Closed JKRhb closed 1 year ago

JKRhb commented 1 year ago

This PR proposes a rework of the uri field within the CoapRequest class. Before, the Uri was constructed by using individual Uri-Host, Uri-Path, Uri-Port, and Uri-Query options.

However, I noticed that a request's URI can be set in a much simpler way by simply turning it into a regular field which can then be converted into and from individual URI component options. This enabled me to implement the algorithms described in section 6.4 and section 6.5 of RFC 7252 in two functions, which makes debugging a bit easier. With these changes, the conversion should now be RFC-compliant (again) and slightly easier to maintain.

The new approach also allowed/incentivized me to make a slight (but breaking) API change, adding the URI as a request constructor parameter. Therefore, this PR can also be seen as an intermediate step towards #160, but also towards further API and type-safety improvements.

(CC @shamblett @JosefWN)

JKRhb commented 1 year ago

I also added a rework for the location options of the CoapResponse class in the latest commits, reusing code from the URI options. This part of the implementation should now also be a bit more RFC-compliant.

JKRhb commented 1 year ago

There are still some details from RFC 7252 that should be addressed later, but I think for now this PR should be ready for review/merging.

codecov-commenter commented 1 year ago

Codecov Report

Base: 30.29% // Head: 29.64% // Decreases project coverage by -0.65% :warning:

Coverage data is based on head (c5a19ac) compared to base (bc091d7). Patch coverage: 50.96% of modified lines in pull request are covered.

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #169 +/- ## ========================================== - Coverage 30.29% 29.64% -0.65% ========================================== Files 61 62 +1 Lines 2793 2783 -10 ========================================== - Hits 846 825 -21 - Misses 1947 1958 +11 ``` | [Impacted Files](https://codecov.io/gh/shamblett/coap/pull/169?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Steve+Hamblett) | Coverage Δ | | |---|---|---| | [lib/src/coap\_client.dart](https://codecov.io/gh/shamblett/coap/pull/169?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Steve+Hamblett#diff-bGliL3NyYy9jb2FwX2NsaWVudC5kYXJ0) | `0.00% <0.00%> (ø)` | | | [lib/src/coap\_observe\_client\_relation.dart](https://codecov.io/gh/shamblett/coap/pull/169?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Steve+Hamblett#diff-bGliL3NyYy9jb2FwX29ic2VydmVfY2xpZW50X3JlbGF0aW9uLmRhcnQ=) | `0.00% <0.00%> (ø)` | | | [lib/src/network/coap\_network\_openssl.dart](https://codecov.io/gh/shamblett/coap/pull/169?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Steve+Hamblett#diff-bGliL3NyYy9uZXR3b3JrL2NvYXBfbmV0d29ya19vcGVuc3NsLmRhcnQ=) | `0.00% <0.00%> (ø)` | | | [lib/src/network/coap\_network\_udp.dart](https://codecov.io/gh/shamblett/coap/pull/169?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Steve+Hamblett#diff-bGliL3NyYy9uZXR3b3JrL2NvYXBfbmV0d29ya191ZHAuZGFydA==) | `0.00% <0.00%> (ø)` | | | [lib/src/stack/layers/blockwise.dart](https://codecov.io/gh/shamblett/coap/pull/169?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Steve+Hamblett#diff-bGliL3NyYy9zdGFjay9sYXllcnMvYmxvY2t3aXNlLmRhcnQ=) | `0.00% <0.00%> (ø)` | | | [lib/src/stack/layers/observe.dart](https://codecov.io/gh/shamblett/coap/pull/169?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Steve+Hamblett#diff-bGliL3NyYy9zdGFjay9sYXllcnMvb2JzZXJ2ZS5kYXJ0) | `0.00% <0.00%> (ø)` | | | [lib/src/coap\_request.dart](https://codecov.io/gh/shamblett/coap/pull/169?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Steve+Hamblett#diff-bGliL3NyYy9jb2FwX3JlcXVlc3QuZGFydA==) | `35.29% <17.64%> (-20.38%)` | :arrow_down: | | [lib/src/codec/udp/message\_encoder.dart](https://codecov.io/gh/shamblett/coap/pull/169?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Steve+Hamblett#diff-bGliL3NyYy9jb2RlYy91ZHAvbWVzc2FnZV9lbmNvZGVyLmRhcnQ=) | `79.41% <50.00%> (+0.30%)` | :arrow_up: | | [lib/src/coap\_response.dart](https://codecov.io/gh/shamblett/coap/pull/169?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Steve+Hamblett#diff-bGliL3NyYy9jb2FwX3Jlc3BvbnNlLmRhcnQ=) | `26.92% <62.50%> (-43.08%)` | :arrow_down: | | [lib/src/option/uri\_converters.dart](https://codecov.io/gh/shamblett/coap/pull/169?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Steve+Hamblett#diff-bGliL3NyYy9vcHRpb24vdXJpX2NvbnZlcnRlcnMuZGFydA==) | `67.12% <67.12%> (ø)` | | | ... and [6 more](https://codecov.io/gh/shamblett/coap/pull/169?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Steve+Hamblett) | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Steve+Hamblett). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Steve+Hamblett)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

JKRhb commented 1 year ago

@shamblett Did you have the chance to look at this PR yet? I think merging it would pave the way for a lot of further improvements. #170, #172, and #174 would also be ready to be merged but might be a bit simpler to review than this one.

shamblett commented 1 year ago

Yep OK merged, the discover_resources and get_blockwise examples seem to be hanging now when I run them, this may be my machine here, I'll have a closer look tomorrow.

JKRhb commented 1 year ago

Yep OK merged, the discover_resources and get_blockwise examples seem to be hanging now when I run them, this may be my machine here, I'll have a closer look tomorrow.

Thank you! But you are right, there seems to be a blockwise issue which I haven't noticed :( I try to come up with a fix as soon as possible :)

JKRhb commented 1 year ago

Found the cause of the issue and opened #175 as a quick fix.