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

Stability fix, keepalive disconnection #368

Merged
merged 2 commits into from
Nov 17, 2022
Merged

Stability fix, keepalive disconnection #368

merged 2 commits into from
Nov 17, 2022

Conversation

brentru
Copy link
Member

@brentru brentru commented Nov 17, 2022

WipperSnapper devices are being disconnected/reconnected every ~120000ms.

This is due to two issues fixed by this PR:

  1. Rigid timing requirement around the MQTT KeepAlive time (exactly every KAT interval)
  2. Blinking the status LED every STATUS_LED_KAT_BLINK_TIMEms causes a blocking behavior that exceeds the client's configured MQTT KeepAlive time, causing the next PINGREQ to execute out-of-bounds

Pre-PR Behavior, note the delta of 6.019 seconds between pings caused by the status LED blinking.

14:37:07.902 -> [235985][V][ssl_client.cpp:369] send_ssl_data(): Writing HTTP request with 2 bytes...
14:37:11.952 -> PING!
14:37:11.952 -> [240019][V][ssl_client.cpp:369] send_ssl_data(): Writing HTTP request with 2 bytes...
14:37:14.963 -> Blinking status LED STATUS_LED_KAT_BLINK_TIME
14:37:17.971 -> PING!
14:37:17.971 -> [246044][V][ssl_client.cpp:369] send_ssl_data(): Writing HTTP request with 2 bytes...
14:37:17.971 -> [246058][V][ssl_client.cpp:324] stop_ssl_socket(): Cleaning SSL connection.
14:37:18.004 -> FSM_NET_ESTABLISH_MQTT

Post-PR behavior

15:13:06.583 -> [962488][V][ssl_client.cpp:369] send_ssl_data(): Writing HTTP request with 2 bytes...
15:13:06.999 -> Blinking status LED STATUS_LED_KAT_BLINK_TIME
15:13:10.609 -> PING!
15:13:10.609 -> [966523][V][ssl_client.cpp:369] send_ssl_data(): Writing HTTP request with 2 bytes...
15:13:14.647 -> PING!

This pull request:

  • Shortens the execution time of statusLEDBlink()
  • Loosens timing requirement within pingBroker()

@brentru brentru requested a review from lorennorman November 17, 2022 20:15
Copy link
Contributor

@lorennorman lorennorman left a comment

Choose a reason for hiding this comment

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

looks perfect! 💯

@brentru brentru merged commit b49c3c3 into adafruit:main Nov 17, 2022
@brentru brentru deleted the fix-blink-timeout branch November 17, 2022 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants