Fix websocket connection sensor#22923
Conversation
homeassistant
added
cla-signed
core
integration: websocket_api
small-pr
ghost
assigned
|
Hey there @home-assistant/core, mind taking a look at this pull request as its been labeled with a integration ( |
Swamp-Ig
force-pushed
the
28890-websocket_api_sensor
branch
from
ca9293d to
1964c61
Compare
Codecov Report
@@ Coverage Diff @@
## dev #22923 +/- ##
==========================================
- Coverage 93.83% 93.83% -0.01%
==========================================
Files 448 448
Lines 36583 36585 +2
==========================================
+ Hits 34329 34330 +1
- Misses 2254 2255 +1
Continue to review full report at Codecov.
|
|
|
||
| self._logger.debug("Received %s", msg) | ||
| connection = await auth.async_handle(msg) | ||
| self._connections.add(connection) |
There was a problem hiding this comment.
let's not add the connection, but instead store id(self), which we also use for logging.
There was a problem hiding this comment.
That will work for this PR, although I was thinking of having a list of connected users in the dev-info page or somewhere, in which case keeping the connection set is needed.
There was a problem hiding this comment.
I think that it would be overkill really. Just because we can get all the data, doesn't mean we should 😆
There was a problem hiding this comment.
In which case it's just as well to keep a bare count, rather than a set of anything.
There was a problem hiding this comment.
How this even work? WebSocketHandler is not a singleton.
There was a problem hiding this comment.
Ahh then it won't work! Maybe we should have just deleted the thing in the first place? Will fix properly.
|
That should fix it now. |
|
The fix didn't work. |
|
Removed from milestone because it is already released |
|
So that means that the fix doesn't work if it has already been released prior to 0.92.2 |
6 participants
Description:
Bug shows that websocket sensor gets out of sync and shows negative numbers. I suspect this is because there's connections made before the websocket sensor is started up.
Fixed by:
hass.dataThe set approach has the advantage that it we can look at the list of connections somewhere else, for example to display the list of current connections somewhere in the API if we choose to at some stage.
Related issue (if applicable): fixes #22890
Checklist:
tox. Your PR cannot be merged unless tests passIf the code does not interact with devices: