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

timout 1 instead of 0 #228

Merged
merged 3 commits into from
Nov 25, 2024
Merged

timout 1 instead of 0 #228

merged 3 commits into from
Nov 25, 2024

Conversation

FoamyGuy
Copy link
Contributor

@ladyada
Resolves: #227

Solution discussed with @brentru on discord. I did try changing the cpython examples to use 0, but that still leads to the exception being raised about being shorter than the socket timeout, so I've left them alone in this PR.

Copy link
Contributor

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

Suggestion:
The current code is:

        if timeout < self._socket_timeout:
            raise MMQTTException(
                f"loop timeout ({timeout}) must be bigger "
                + f"than socket timeout ({self._socket_timeout}))"
            )

The test is actually <, not <=, so a more precise message would be ... must be >= ... instead of ... must be bigger ....

Now, in terms of actually working properly, should the loop timeout be absolutely greater than the socket timeout, or just greater or equal?

query for @brentru

@brentru
Copy link
Member

brentru commented Nov 22, 2024

should the loop timeout be absolutely greater than the socket timeout, or just greater or equal?

Greater or equal

@FoamyGuy
Copy link
Contributor Author

Thank you @dhalbert and @brentru, the message is updated with the latest commit to >= instead of bigger

adafruit_minimqtt/adafruit_minimqtt.py Outdated Show resolved Hide resolved
@FoamyGuy FoamyGuy merged commit 76f8c28 into adafruit:main Nov 25, 2024
1 check passed
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Nov 26, 2024
Updating https://github.com/adafruit/Adafruit_CircuitPython_Ducky to 1.5.0 from 1.1.4:
  > Merge pull request adafruit/Adafruit_CircuitPython_Ducky#19 from FoamyGuy/ms_instead_of_ns

Updating https://github.com/adafruit/Adafruit_CircuitPython_MiniMQTT to 7.10.5 from 7.10.4:
  > Merge pull request adafruit/Adafruit_CircuitPython_MiniMQTT#228 from FoamyGuy/timeout_fix

Updating https://github.com/adafruit/Adafruit_CircuitPython_Bundle/circuitpython_library_list.md to NA from NA:
  > Updated download stats for the libraries
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.

Loop default timeout raises exception
3 participants