HASwitchPlate / openHASP-custom-component

Home Assistant custom component for openHASP
https://www.openhasp.com
MIT License
51 stars 12 forks source link

Dynamic reloading broken on recently added displays. #140

Closed xNUTx closed 3 months ago

xNUTx commented 3 months ago

The display stops or refuses to dynamically update (from HA) and throws an error in the logs because (most likely) num_pages contains a string, while page contains an int, resulting in a python type error.

dgomes commented 3 months ago

Could you please provide a log of the error ? I would prefer to trace to error to the source then to fix where we find the symptom

xNUTx commented 3 months ago

Exception in lwt_message_received when handling msg on 'hasp/seeeddisplay1/LWT': 'online' Traceback (most recent call last): File "/config/custom_components/openhasp/__init__.py", line 478, in lwt_message_received await self.async_load_page(self._pages_jsonl) File "/config/custom_components/openhasp/__init__.py", line 673, in async_load_page await self.refresh() File "/config/custom_components/openhasp/__init__.py", line 628, in refresh await self.async_change_page(self._page) File "/config/custom_components/openhasp/__init__.py", line 559, in async_change_page if page <= 0 or page > num_pages: ^^^^^^^^^^^^^^^^ TypeError: '>' not supported between instances of 'int' and 'str'

I reintroduced the bug but added a comment with the actual fix. the line numbers beyond 559 are +1 instead of the actual lines in the trace.

FreeBear-nc commented 3 months ago

Page is already being tested against zero (so should already be a number). Does it really need to be cast to an int for the test against num_pages ?

xNUTx commented 3 months ago

As you can see in the error log, it seems to be able to have more then just an int, so the change helps to fix this.

~I was not sure where this string content comes from to fix the source issue, which may be better to fix instead. Also I am unsure if it was the 'page' or the 'num_pages' variable carrying the string... but if you say 'page' is already checked for number content elsewhere, I think it must have been 'num_pages'.~

See https://github.com/HASwitchPlate/openHASP-custom-component/blob/86bca7ecd77ccb163084d66dc2724a34b23bdb40/custom_components/openhasp/__init__.py#L539C1-L561C23

It shows 'page' can hold a string called 'all'. There is no type cast on 'num_pages' when it is filled.

xNUTx commented 3 months ago

Skipping through the code in the component, I can conclude BOTH 'page' and 'num_pages' hold a string initially.

https://github.com/HASwitchPlate/openHASP-custom-component/blob/86bca7ecd77ccb163084d66dc2724a34b23bdb40/custom_components/openhasp/__init__.py#L338

HASP_NUM_PAGES is set with the discovered number of pages here:

https://github.com/HASwitchPlate/openHASP-custom-component/blob/86bca7ecd77ccb163084d66dc2724a34b23bdb40/custom_components/openhasp/config_flow.py#L151

We can actually conclude this is pretty much guaranteed an int as HASP_NUM_PAGES in self._statusupdate is set using the discovered 'CONF_PAGES'.

So it is 'page' that can hold the string 'all' which does not allow a 'larger then' int comparison. That is the one which needs a workaround in that check.

xNUTx commented 3 months ago

Updated the code to check 'page' for the type int instead of casting both to it.

This should satisfy the check in every possible way and not throw exceptions.

xNUTx commented 2 months ago
Logger: homeassistant.util.logging
Bron: util/logging.py:95
Eerst voorgekomen: 17:33:32 (6 gebeurtenissen)
Laatst gelogd: 17:34:09

Exception in lwt_message_received when handling msg on 'hasp/seeeddisplay3/LWT': 'online' Traceback (most recent call last): File "/config/custom_components/openhasp/__init__.py", line 478, in lwt_message_received await self.async_load_page(self._pages_jsonl) File "/config/custom_components/openhasp/__init__.py", line 671, in async_load_page await self.refresh() File "/config/custom_components/openhasp/__init__.py", line 626, in refresh await self.async_change_page(self._page) File "/config/custom_components/openhasp/__init__.py", line 559, in async_change_page if isinstance(page, int) and (page <= 0 or page > num_pages): ^^^^^^^^^^^^^^^^ TypeError: '>' not supported between instances of 'int' and 'str'
Exception in lwt_message_received when handling msg on 'hasp/seeeddisplay1/LWT': 'online' Traceback (most recent call last): File "/config/custom_components/openhasp/__init__.py", line 478, in lwt_message_received await self.async_load_page(self._pages_jsonl) File "/config/custom_components/openhasp/__init__.py", line 671, in async_load_page await self.refresh() File "/config/custom_components/openhasp/__init__.py", line 626, in refresh await self.async_change_page(self._page) File "/config/custom_components/openhasp/__init__.py", line 559, in async_change_page if isinstance(page, int) and (page <= 0 or page > num_pages): ^^^^^^^^^^^^^^^^ TypeError: '>' not supported between instances of 'int' and 'str'
Exception in lwt_message_received when handling msg on 'hasp/seeeddisplay2/LWT': 'online' Traceback (most recent call last): File "/config/custom_components/openhasp/__init__.py", line 478, in lwt_message_received await self.async_load_page(self._pages_jsonl) File "/config/custom_components/openhasp/__init__.py", line 671, in async_load_page await self.refresh() File "/config/custom_components/openhasp/__init__.py", line 626, in refresh await self.async_change_page(self._page) File "/config/custom_components/openhasp/__init__.py", line 559, in async_change_page if isinstance(page, int) and (page <= 0 or page > num_pages): ^^^^^^^^^^^^^^^^ TypeError: '>' not supported between instances of 'int' and 'str'

The fix seems to not be complete. Somehow num_pages can hold a string as well... :thinking: