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

Fix to update the timestamp of the last packet received #235

Merged
merged 2 commits into from
Dec 15, 2022
Merged

Fix to update the timestamp of the last packet received #235

merged 2 commits into from
Dec 15, 2022

Conversation

onkwon
Copy link
Contributor

@onkwon onkwon commented Dec 2, 2022

It doesn't look like pContext->lastPacketRxTime getting updated anywhere. And once PACKET_RX_TIMEOUT_MS reached the condition will always be true.

@jasonpcarroll
Copy link
Member

Hi @onkwon,

Sorry for seeing this so late! Thank you for bringing this to our attention. I will take a look and notify other team members of this issue.

Best,

Jason Carroll

@onkwon
Copy link
Contributor Author

onkwon commented Dec 7, 2022

Please note that this would cause customer bill increase as ping-pong every MQTT_ProcessLoop() after PACKET_RX_TIMEOUT_MS.

@archigup
Copy link
Member

archigup commented Dec 7, 2022

Hey, this PR needs a change in docs/doxygen/include/size_table.md (see here). If you'd prefer, I can add the change to this PR for you.

Otherwise, PR looks good to me

Copy link
Member

@AniruddhaKanhere AniruddhaKanhere left a comment

Choose a reason for hiding this comment

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

Thanks @onkwon for this PR. All looks good and I am approving this.

source/core_mqtt.c Outdated Show resolved Hide resolved
@onkwon onkwon requested a review from paulbartell December 9, 2022 00:32
Co-authored-by: Paul Bartell <[email protected]>
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.

5 participants