awslabs / aws-c-mqtt

C99 implementation of the MQTT 3.1.1 specification.
Apache License 2.0
93 stars 29 forks source link

Mqtt311 decoder #311

Closed bretambrose closed 1 year ago

bretambrose commented 1 year ago

While we could have fixed the original bug without doing such a heavyweight refactor, testing complex, brittle logic embedded in the channel handler's vtable implementation is a lousy approach. Rather than graft more hacks and more complexity into the client impl, we add a new subsystem with a testable API.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

codecov-commenter commented 1 year ago

Codecov Report

Patch coverage: 85.33% and project coverage change: +0.59% :tada:

Comparison is base (7aaae0c) 81.10% compared to head (d89aef3) 81.69%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #311 +/- ## ========================================== + Coverage 81.10% 81.69% +0.59% ========================================== Files 18 19 +1 Lines 7842 7923 +81 ========================================== + Hits 6360 6473 +113 + Misses 1482 1450 -32 ``` | [Files Changed](https://app.codecov.io/gh/awslabs/aws-c-mqtt/pull/311?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=awslabs) | Coverage Δ | | |---|---|---| | [source/client\_channel\_handler.c](https://app.codecov.io/gh/awslabs/aws-c-mqtt/pull/311?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=awslabs#diff-c291cmNlL2NsaWVudF9jaGFubmVsX2hhbmRsZXIuYw==) | `73.54% <64.40%> (+4.98%)` | :arrow_up: | | [source/mqtt311\_decoder.c](https://app.codecov.io/gh/awslabs/aws-c-mqtt/pull/311?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=awslabs#diff-c291cmNlL21xdHQzMTFfZGVjb2Rlci5j) | `98.80% <98.80%> (ø)` | | | [source/client.c](https://app.codecov.io/gh/awslabs/aws-c-mqtt/pull/311?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=awslabs#diff-c291cmNlL2NsaWVudC5j) | `69.29% <100.00%> (-0.11%)` | :arrow_down: | | [source/fixed\_header.c](https://app.codecov.io/gh/awslabs/aws-c-mqtt/pull/311?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=awslabs#diff-c291cmNlL2ZpeGVkX2hlYWRlci5j) | `87.93% <100.00%> (+12.06%)` | :arrow_up: | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/awslabs/aws-c-mqtt/pull/311/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=awslabs)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

bretambrose commented 1 year ago

https://www.youtube.com/watch?v=NoDsJAiseGg

Even better: https://www.youtube.com/watch?v=mjfT2joaJxs