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

Fix is_connected property when not using loop_forever #795

Merged
merged 4 commits into from
Jan 20, 2024
Merged

Fix is_connected property when not using loop_forever #795

merged 4 commits into from
Jan 20, 2024

Conversation

PierreF
Copy link
Contributor

@PierreF PierreF commented Jan 7, 2024

Fix #525

@PierreF PierreF mentioned this pull request Jan 7, 2024
Closed
akx
akx reviewed Jan 10, 2024
View reviewed changes
Copy link
Contributor

@akx akx left a comment

Choose a reason for hiding this comment

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

I think there are stray logger initializations and log calls in here :)

A test would probably not go amiss either.

src/paho/mqtt/client.py Outdated
Comment on lines 604 to 605
logging.basicConfig(level=logging.DEBUG)
self._logger = logging.getLogger("plop")
Copy link
Contributor

Choose a reason for hiding this comment

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

plop! 😂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, totally missed this temporary debugging code.

akx
akx suggested changes Jan 11, 2024
View reviewed changes
Copy link
Contributor

@akx akx left a comment

Choose a reason for hiding this comment

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

There's a couple more debug log calls remaining, as well as what I think may be a stray loop_stop() (in a more perfect world, we'd use e.g. py.test fixtures or a with block to manage loop_stop() always getting called, but that's out of scope for this PR)

src/paho/mqtt/client.py Outdated
@@ -1762,6 +1771,8 @@ def loop_misc(self) -> MQTTErrorCode:
if self._state == mqtt_cs_disconnecting:
rc = MQTTErrorCode.MQTT_ERR_SUCCESS
else:
self._state = ConnectionState.MQTT_CS_CONNECTION_LOST
self._easy_log(MQTT_LOG_DEBUG, "... 2")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
self._easy_log(MQTT_LOG_DEBUG, "... 2")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad, I should re-read my own PR more carefully :(

src/paho/mqtt/client.py Outdated
@@ -2574,6 +2585,10 @@ def _loop_rc_handle(

self._do_on_disconnect(rc, properties)

if rc == MQTT_ERR_CONN_LOST:
self._easy_log(MQTT_LOG_DEBUG, "... 3")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
self._easy_log(MQTT_LOG_DEBUG, "... 3")

tests/test_client.py Outdated
Comment on lines 249 to 250
finally:
mqttc.loop_stop()
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see loop_start being called in this test..?

@PierreF PierreF merged commit fb42d41 into master Jan 20, 2024
17 of 21 checks passed
@PierreF PierreF deleted the fix_is_connected branch January 20, 2024 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akx akx akx requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

State of client doesn't change to disconnected after connection lost
2 participants
@PierreF @akx