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

Make non-top-level connection errors debug level #361

Merged
merged 2 commits into from
Dec 2, 2024

Conversation

bobrippling
Copy link

This avoids spamming the home-assistant logs, which can occur when home-assistant has been running but the roomba hasn't been turned on, so no initial connection has been established.
This allows users to read the logs/investigate other home-assistant issues, without having to disable the roomba integration.

For example instead of this message being repeated 3x:

2024-MM-DD HH:MM:SS.MMM ERROR (roombapy) [roombapy.remote_client] Can't connect to 192.168.0.123
Traceback (most recent call last):
  File .../lib/python3.12/site-packages/roombapy/remote_client.py", line 93, in connect
    self._open_mqtt_connection()
  File .../lib/python3.12/site-packages/roombapy/remote_client.py", line 117, in _open_mqtt_connection
    self.mqtt_client.connect(self.address, self.port)
  File .../lib/python3.12/site-packages/paho/mqtt/client.py", line 914, in connect
    return self.reconnect()
           ^^^^^^^^^^^^^^^^
  File .../lib/python3.12/site-packages/paho/mqtt/client.py", line 1044, in reconnect
    sock = self._create_socket_connection()
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File .../lib/python3.12/site-packages/paho/mqtt/client.py", line 3685, in _create_socket_connection
    return socket.create_connection(addr, timeout=self._connect_timeout, source_address=source)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File .../lib/python3.12/socket.py", line 865, in create_connection
    raise exceptions[0]
  File .../lib/python3.12/socket.py", line 850, in create_connection
    sock.connect(sa)
OSError: [Errno 113] No route to host

and these then following:

2024-MM-DD HH:MM:SS.MMM ERROR (roombapy) [roombapy.remote_client] Unable to connect to 192.168.0.123
2024-MM-DD HH:MM:SS.MMM WARNING (roombapy) [roombapy.roomba] Unexpectedly disconnected from Roomba 192.168.0.123, code The connection was lost
2024-MM-DD HH:MM:SS.MMM WARNING (roombapy) [roombapy.roomba] Periodic connection lost due to Unable to connect to Roomba at 192.168.0.123

we now receive:

2024-MM-DD HH:MM:SS.MMM WARNING (roombapy) [roombapy.roomba] Unexpectedly disconnected from Roomba 192.168.0.123, code The connection was lost

With the ability to get back more detailed connection logs by enabling debug logging in home-assistant.

@bobrippling
Copy link
Author

Rebased

@bobrippling
Copy link
Author

The build error is on unchanged code - looks like it's been picked up by looking at context around the changes

@jasperslits
Copy link

I noticed this PR as I was about to create one myself after seeing the log spam when my Roomba had an empty battery after being stuck. Current main branch does not return any ruff errors (ruff 0.8.1) with the latest commits from past couple of days, so a rebase should make the unrelated errors go away.

However, it seems the only Exception returned there is OSError so it might be cleaner to address that.

But even if the logging is changed from exception to debug, it's still spamming the logs up to 3 times with no added value compared to this

self.log.error("Unable to connect to %s", self.address) 

one at the end of the connect method. No?

@bobrippling
Copy link
Author

[...] so a rebase should make the unrelated errors go away.

Thanks for the prompt - rebased

However, it seems the only Exception returned there is OSError so it might be cleaner to address that.

Nice idea - sorted in #382

But even if the logging is changed from exception to debug, it's still spamming the logs up to 3 times with no added value compared to [...] at the end of the connect method. No?

Agreed - I wanted to keep this change minimal to increase its chances of being accepted - however if the larger change (with other debug lines removed) would be accepted, I'll make the change. @pschmitt wdyt?

This avoids spamming the home-assistant logs, which can occur when
home-assistant has been running but the roomba hasn't been turned on, so
no initial connection has been established.
This allows users to read the logs/investigate other home-assistant
issues, without having to disable the roomba integration.

For example instead of this message being repeated 3x:

```
2024-MM-DD HH:MM:SS.MMM ERROR (roombapy) [roombapy.remote_client] Can't connect to 192.168.0.123
Traceback (most recent call last):
  File .../lib/python3.12/site-packages/roombapy/remote_client.py", line 93, in connect
    self._open_mqtt_connection()
  File .../lib/python3.12/site-packages/roombapy/remote_client.py", line 117, in _open_mqtt_connection
    self.mqtt_client.connect(self.address, self.port)
  File .../lib/python3.12/site-packages/paho/mqtt/client.py", line 914, in connect
    return self.reconnect()
           ^^^^^^^^^^^^^^^^
  File .../lib/python3.12/site-packages/paho/mqtt/client.py", line 1044, in reconnect
    sock = self._create_socket_connection()
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File .../lib/python3.12/site-packages/paho/mqtt/client.py", line 3685, in _create_socket_connection
    return socket.create_connection(addr, timeout=self._connect_timeout, source_address=source)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File .../lib/python3.12/socket.py", line 865, in create_connection
    raise exceptions[0]
  File .../lib/python3.12/socket.py", line 850, in create_connection
    sock.connect(sa)
OSError: [Errno 113] No route to host
```

and these then following:

```
2024-MM-DD HH:MM:SS.MMM ERROR (roombapy) [roombapy.remote_client] Unable to connect to 192.168.0.123
2024-MM-DD HH:MM:SS.MMM WARNING (roombapy) [roombapy.roomba] Unexpectedly disconnected from Roomba 192.168.0.123, code The connection was lost
2024-MM-DD HH:MM:SS.MMM WARNING (roombapy) [roombapy.roomba] Periodic connection lost due to Unable to connect to Roomba at 192.168.0.123
```

we now receive:
```
2024-MM-DD HH:MM:SS.MMM WARNING (roombapy) [roombapy.roomba] Unexpectedly disconnected from Roomba 192.168.0.123, code The connection was lost
```

With the ability to get back more detailed connection logs by enabling debug logging in home-assistant.
@pschmitt
Copy link
Owner

pschmitt commented Dec 2, 2024

all right let's do this :)

@pschmitt pschmitt merged commit 5721678 into pschmitt:main Dec 2, 2024
6 checks passed
@bobrippling bobrippling deleted the feat/quieter-logging branch December 2, 2024 08:45
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.

3 participants