Skip to content

handleSubscriptionPacket - topic calc error#233

Merged
tyeth merged 3 commits intomasterfrom
packet-len-calc
Sep 3, 2025
Merged

handleSubscriptionPacket - topic calc error#233
tyeth merged 3 commits intomasterfrom
packet-len-calc

Conversation

@tyeth
Copy link
Copy Markdown
Member

@tyeth tyeth commented Aug 29, 2025

This updates the topic length and start calculation according to the mqtt3.1 spec, it turns out there's a similar calculation in the readFullPacket code (L297).

@brentru
Copy link
Copy Markdown
Member

brentru commented Sep 2, 2025

@tyeth Have you tested this on IO Prod? Have you tested it with topics other than I2C?

Copy link
Copy Markdown
Member

@brentru brentru left a comment

Choose a reason for hiding this comment

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

This looks good and addresses 2.2.3 of the MQTT 3.1.1 spec. Additionally, after glancing at your PRs, it looks like the bug has been fixed, which is great.

I am requesting one change, please tag me when done

Comment thread Adafruit_MQTT.cpp
tyeth added a commit to adafruit/Adafruit_Wippersnapper_Arduino that referenced this pull request Sep 3, 2025
@tyeth
Copy link
Copy Markdown
Member Author

tyeth commented Sep 3, 2025

@tyeth Have you tested this on IO Prod? Have you tested it with topics other than I2C?

Just to confirm, yes I did test it with some other components being added. Again from CI this morning, against Prod, adding neopixel strands to a qtpy.

Copy link
Copy Markdown
Member

@brentru brentru left a comment

Choose a reason for hiding this comment

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

@tyeth I see that you've tested this, LGTM and release after merging so WS CI can pick it up

@tyeth tyeth merged commit 10c5d51 into master Sep 3, 2025
2 checks passed
@tyeth tyeth deleted the packet-len-calc branch September 3, 2025 14:46
@tyeth tyeth restored the packet-len-calc branch September 3, 2025 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants