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.

Sign up for GitHub

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

Jump to bottom

Keep Alive has an arbitrary limit #643

Closed
danieldougherty opened this issue Jun 15, 2023 · 1 comment
Closed

Keep Alive has an arbitrary limit #643

danieldougherty opened this issue Jun 15, 2023 · 1 comment
Labels
good first issue Good for newcomers stale Not moving forward; blocked

Comments

@danieldougherty
Copy link
Contributor

Expected Behavior

According to both MQTT specs, the Keep Alive value should be an integer from 0 to 65,535. A non-zero value indicates the number of seconds between messages before the client must send a pingreq; a value of 0 disables the keep-alive mechanism.

Current Behavior

Attempting to set a keep-alive value of less than 5 seconds fails with the output 'Keep alives should be >= 5 secs'. This makes it impossible to set a 0 value, or to use shorter intervals for testing/time-sensitive applications.
@h3nill
Copy link

h3nill commented Jun 17, 2023

currently pings are sent regardless of if there is activity on the network or not.

So to avoid user from setting it something very small and generate alot of unnecessary traffic we had put that limit. Also according to spec:

The actual value of the Keep Alive is application specific; typically this is a few minutes.

But I agree it should be >= 1 second.

it impossible to set a 0 value

i think this can be implemented by disabling the select! branch that sends the pings (cc: @yatinmaan)

Both of this should be an easy fix, do you want to open a PR for this @danieldougherty ?

@h3nill h3nill added the good first issue Good for newcomers label Jun 17, 2023
danieldougherty added a commit to danieldougherty/rumqtt that referenced this issue Jul 3, 2023
@danieldougherty
fix(Event Loop): Allow keep-alives <= 5 secs
4c7d639 
This PR removes the requirement that the keep alive timeout be at
least 5 seconds. This is in keeping with the MQTT spec, which
states that the keep alive value should be between 0 and 65535.
A value of 0 indicates that no keep-alive pings are sent to the
broker.

Issue: bytebeamio#643 - Keep Alive has an arbitrary limit
@danieldougherty danieldougherty mentioned this issue Jul 3, 2023
Merged
@stale stale bot added the stale Not moving forward; blocked label Jul 9, 2023
h3nill pushed a commit that referenced this issue Aug 20, 2023
@danieldougherty
fix(Event Loop): Allow keep-alives <= 5 secs (#655)
3b8cac5 
This PR removes the requirement that the keep alive timeout be at
least 5 seconds. This is in keeping with the MQTT spec, which
states that the keep alive value should be between 0 and 65535.
A value of 0 indicates that no keep-alive pings are sent to the
broker.

Issue: #643 - Keep Alive has an arbitrary limit
@h3nill h3nill closed this as completed Aug 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Good for newcomers stale Not moving forward; blocked
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@h3nill @danieldougherty