Skip to content

Bugfix: Invalid function called#58

Open
wennergr wants to merge 1 commit into
puttyman:masterfrom
wennergr:wennergr/bugfix-unload
Open

Bugfix: Invalid function called#58
wennergr wants to merge 1 commit into
puttyman:masterfrom
wennergr:wennergr/bugfix-unload

Conversation

@wennergr
Copy link
Copy Markdown

Failed to unload due to calling a non existing function

Failed to unload due to calling a non existing function
@hawksj
Copy link
Copy Markdown
Collaborator

hawksj commented Nov 30, 2024

Hi Tobias,

Thanks for the PR! Please could you provide steps to replicate the bug that this PR fixes please? Then we can test some before/after.

Thanks,
Sam

@wennergr
Copy link
Copy Markdown
Author

wennergr commented Dec 1, 2024

  1. Update configuration (such as enabling a sensor)
  2. Cause a reload event (such as: ha core restart)
  3. Check logs from (home-assistant.log.1 if restarted and forced a log to rotate)
2024-12-01 13:58:57.586 INFO (MainThread) [homeassistant.config_entries] Reloading configuration entries because disabled_by changed in entity registry: <retrated>
2024-12-01 13:58:57.588 INFO (MainThread) [custom_components.amplifi] Stopping coordinator
2024-12-01 13:58:57.589 ERROR (MainThread) [homeassistant.config_entries] Error unloading entry <retracted> for amplifi
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/config_entries.py", line 856, in async_unload
    await self._async_process_on_unload(hass)
  File "/usr/src/homeassistant/homeassistant/config_entries.py", line 1034, in _async_process_on_unload
    if job := self._on_unload.pop()():
              ^^^^^^^^^^^^^^^^^^^^^^^
  File "/config/custom_components/amplifi/__init__.py", line 42, in async_stop_coordinator
    coordinator._async_stop_refresh(None)
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'AmplifiDataUpdateCoordinator' object has no attribute '_async_stop_refresh'. Did you mean: 'async_stop_refresh'?

It looks like you want to call the async_stop_refresh function (that takes no argument). This function calls ._async_stop_refresh in the super class.

I assume this is what you want to do as there is no function called _async_stop_refresh that takes one argument on an instance of AmplifiDataUpdateCoordinator

@hawksj
Copy link
Copy Markdown
Collaborator

hawksj commented Dec 2, 2024

Hi Tobias,

Thanks for this. If I'm reading this output correctly, I believe this pushes the error onto the coordinator module:

Logger: homeassistant.config_entries
Source: config_entries.py:856
First occurred: 9:01:50 AM (1 occurrences)
Last logged: 9:01:50 AM

Error unloading entry 172.20.0.1 for amplifi
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/config_entries.py", line 856, in async_unload
    await self._async_process_on_unload(hass)
  File "/usr/src/homeassistant/homeassistant/config_entries.py", line 1034, in _async_process_on_unload
    if job := self._on_unload.pop()():
              ^^^^^^^^^^^^^^^^^^^^^^^
  File "/config/custom_components/amplifi/__init__.py", line 41, in async_stop_coordinator
    coordinator.async_stop_refresh()
  File "/config/custom_components/amplifi/coordinator.py", line 154, in async_stop_refresh
    super._async_stop_refresh()
    ^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: type object 'super' has no attribute '_async_stop_refresh'

Changing line 154 in coordinator.py to super.async_stop_refresh() doesn't resolve this new error.

The original author of this repo isn't active any more as they don't use the hardware. @atudor2 and I are most active in here now, but my knowledge of Python OOB is very limited. Perhaps Alistair will understand what is going on here better and may be able to assist.

I will avoid merging this until this function works as expected. If the underlying issue remains unresolved, I am not keen to change things unnecessarily. In the meantime, restarting Home Assistant when the integration dies does bring the entities back in my experience. If you do find a fix, please push the changes to the same branch so we can test and hopefully merge in the future.

Thanks,
Sam

@hawksj
Copy link
Copy Markdown
Collaborator

hawksj commented Dec 2, 2024

I've done some searching and found that this function no longer exists in the Home Assistant core. It was removed in this commit
home-assistant/core@8910d26 which was released in 2022.7.

It was removed with the note "This avoids the ambuigity as to what happens if same callback is added multiple times."

In that commit, in test_update_coordinator.py the async_remove_listener function was replaced with unsub which appears to have been defined as crd.async_add_listener(update_callback). I think crd in that example is the coordinator object. I'm not really sure what's going on here, maybe you don't need the stop_refresh function any more? I'm not sure but this might mean that the function that creates the listeners might need to be updated to reflect these changes too?

@wennergr
Copy link
Copy Markdown
Author

wennergr commented Dec 4, 2024

Totally. Maybe the right thing to do is to remove it all. I'm going to do some digging into this as well.

I'm a bit new to custom components in home assistant

Cheers!

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.

2 participants