pyhomematic icon indicating copy to clipboard operation
pyhomematic copied to clipboard

HmIPW-DRBL4: entity not availably any more after change of "Betriebsmodus" from "Jalousie" to "Rolllade"

Open ivo7B4 opened this issue 3 years ago • 22 comments

Hello

I've changed for a few channels of a HmIPW-DRBL4 the "Betriebsmodus" from "Jalousie" to "Rolllade".

I still see the entities for the channels which I kept on the "Betriebsmoduls" as "Jalousie".

But I don't have the entitites any more for the channels where I've changed the "Betriebsmodus" to "Rolllade". The are still visible as "restored" only but don't work any more. If I use these restored entities, I cannot use them as I get the following message in the lovelace UI: "This entity is currently unavailable and is an orphan to a removed, changed or dysfunctional integration or device. If the entity is no longer in use, you can clean it up by removing it."

Is the Betriebsmodus "Rolllade" not supported yet? Please let me know if there is anything that I can contribute to solve this issue. (I've not added the JSON files for this device as it is available already.)

Thank you!

ivo7B4 avatar Jan 02 '22 12:01 ivo7B4

pyhomematic is a very static library which doesn't support different channel modes. So this can't be fixed here. That's one of the reaseons why we have started the new integration. It provides more flexibility and should be able to handle your usecase better.

danielperna84 avatar Jan 02 '22 19:01 danielperna84

Thank you @danielperna84 for your explanation. I understand and I look forward to the new integration. Thank you.

ivo7B4 avatar Jan 03 '22 20:01 ivo7B4

Hi @danielperna84, do you have any idea how to identify which mode is configured? Due to the paramters (LEVEL, LEVEL_2) i would identify this as blind.

SukramJ avatar Jan 04 '22 16:01 SukramJ

For old devices it was possible to get this from the MASTER paramset when looking at BEHAVIOR, like we are doing here. I don't find the source which pointed me into that direction back in the day. I don't know how this is implemented for HmIP though. There's no BEHAVIOUR key in the paramset_description. Maybe with HmIP the content of the paramset description actually changes. 🤔

@ivo7B4

Could you compare the paramset description where the device is configured in one way first, then delete the cache files, then configure it to the other mode, then fetch the paramsets again?

danielperna84 avatar Jan 04 '22 18:01 danielperna84

Hi @danielperna84 , I've just tried whether I can find a difference in the paramset description.

I haven't switched on the same device yet as changing the Betriebsmodus would require that I delete before all corresponding "Direktverknüfpungen" and "Programme". But I've exported the paramset description from the device where I have configured as follows:

  • Ch 1 and Ch 5: Jalousie
  • Ch 9 and Ch 13: Rollade

I've also exported the paramset description from another device where all channels are configured as Jalousie. If I compare these paramset files, I don't see any relevant difference (only differences are address information, I've checked with a compare tool to avoid that I miss any relevant difference).

But if there would be no interface difference, why does home assistant recognize that I've switched the Betriebsmodus and does not show me the corresponding entites any more? (as described in my first comment) Could there be thus a difference in another file? Or in a response to a specific request?

If I can try out something else, just let me know.

Thank you.

ivo7B4 avatar Jan 04 '22 19:01 ivo7B4

Can you post the Home Assistant logs which are generated during startup? Maybe there's an error which can indicate what the problem is. In general I don't see what the problem could be.

Support for the device was added by @Knechtie here: https://github.com/danielperna84/pyhomematic/pull/398

Maybe he has an idea?

Edit:

Looking at the implementation, for HmIP the relevant Parameter seems to have been renamed to CHANNEL_OPERATION_MODE, and it can have the value SHUTTER or BLIND. But the implementation here should actually account for this as far as I can tell. 🤔 Here's the parameter in the paramset description.

danielperna84 avatar Jan 04 '22 20:01 danielperna84

I've just checked the log files:

There is an error: Error adding entities for domain cover with platform homematic ... – (ERROR) Cover - message first occurred at ... and shows up 2 times

As I have two channels which I've changed to "Rollladen", I assume that this is the reason why this error shows up 2 times.

When I open this error: Logger: homeassistant.components.cover Source: components/homematic/cover.py:86 Integration: Cover (documentation, issues) First occurred: .... 2 occurrences) Last logged: ....

Error adding entities for domain cover with platform homematic Error while setting up homematic platform for cover Traceback (most recent call last): File "/usr/src/homeassistant/homeassistant/helpers/entity_platform.py", line 382, in async_add_entities await asyncio.gather(*tasks) File "/usr/src/homeassistant/homeassistant/helpers/entity_platform.py", line 523, in _async_add_entity supported_features=entity.supported_features, File "/usr/src/homeassistant/homeassistant/components/cover/init.py", line 269, in supported_features if self.current_cover_tilt_position is not None: File "/usr/src/homeassistant/homeassistant/components/homematic/cover.py", line 86, in current_cover_tilt_position return int(self._data.get("LEVEL_2", 0) * 100) ValueError: invalid literal for int() with base 10: ''

As the mode "Rollladen" does not have a tilt position, I have the impression that the module expects a valid int for the tilt position which is not availble in this mode, thus causing this error. But only an assumption, I did not look into the source code.

Thank you!

ivo7B4 avatar Jan 05 '22 07:01 ivo7B4

I think the problem is actually within the Home Assistant code. Could you please replace this line with:

position = self._data.get("LEVEL_2", 0)
if position is None or position == '':
    return None
return int(position * 100)

And see what this does? For some reason during startup there is no value present for the parameter. I assume this will change during runtime. The question is what effect it will have if we let the tilt position remain at unknown during startup. It might be, that it won't be usable later on.

danielperna84 avatar Jan 08 '22 11:01 danielperna84

Thank you @danielperna84

I have all the HMIPW devices connected, but I'm currently only preparing them for a later installation (e.g. blinds etc. are not connected yet). I'll thus won't be able to check the right behaviour, but I can check the status in the CCU3 what is probably sufficient.

As I currently have home assistant installed on a Raspberry Pi 4 with with home assistant operating system, I don't see an option to get access to the source code files (I've tried Visual Studio, Samba, ....). I assume that I need to install a developer version as described here https://developers.home-assistant.io/docs/add-ons/testing/, is this correct?

If yes, I'll try to get a developer version running that I can test your proposed change.

ivo7B4 avatar Jan 08 '22 12:01 ivo7B4

There are ways to modify the source code when using Home Assistant OS. But I always forget the correct way to do it as I don't use that installation method. Maybe what's described here is sufficient.

In any case, it shouldn't matter if the blinds are not connected. The actors will still turn on and off. So as long as the device is usable in the CCU, it should be usable in Home Assistant as well.

But of course you could use a separate development VM to test this. Might make more sense after all to not mess with your production HA instance.

danielperna84 avatar Jan 08 '22 12:01 danielperna84

Thank you @danielperna84 I've meanwhile also found this link https://developers.home-assistant.io/docs/operating-system/debugging?_highlight=debugging I'll try it as described there. My CCU3 configuration is quite final, there I'm careful. My home assistant installation is mainly still a trial (I'm quite sure I'll stay with home assistant; but I started with trials to test different home automation software) and it's thus no issue if a would need to re-install and re-configure it. Thus, I can do it directly in my current installation.

I'll let you know as soon as I was able to test this and I have the result.

ivo7B4 avatar Jan 08 '22 14:01 ivo7B4

Hi @danielperna84 According to the link above, I've tried to install the development environment. I've installed Docker, VS Code, have clicked on 'fork' as desribed and then opened the new link as described there. If I do that, I always get the error message from Visual Studio that I cannot access the repository. This is not homematic related. This is only for your information why I have not replied yet as I've not been able so far to try it what you've proposed. I continue to try to get it working, maybe also a manual installation. Only FYI, as it probably will still take some time.

ivo7B4 avatar Jan 16 '22 13:01 ivo7B4

Hi @danielperna84 I've switched to another PC and I've go it it running. Yes, your change as you've proposed here: https://github.com/danielperna84/pyhomematic/issues/455#issuecomment-1007964881 seems to fix the issue (I can't tell for sure yet as I'm only preparing the devices for later installation, but):

  • I see all 4 channels with this fix; and
  • I can adjust for the 2 channels that are "Jalousien" both tilt and position and for those two channel that are "Rollade" I can change the position only, as expected.

Again, thank you very much! Will you commit this fix? I would prefer this as this fix is from you and as I still need to get more familiar with the development environment.

ivo7B4 avatar Jan 16 '22 16:01 ivo7B4

I have created the PR. 👍

danielperna84 avatar Jan 16 '22 19:01 danielperna84

Thank you!

ivo7B4 avatar Jan 17 '22 18:01 ivo7B4

Hi @danielperna84

Thank you for your fix again. WIth the new release, it still works for "Rolladen". For "Jalousien", the tilit position is only there when the value is not 0. As soon as the value has been set to 0 once, the tilt position is not there any more and thus, can also not be changed any more. Work-around: Use homematic (e.g. with CCU interface) to set it to a value which is non zero.

When I tested it, this worked perfectly.

My impression: In your proposed fix, it was only tested against none or '': if position is None or position == '': return None

I've looked up the current implementation in https://github.com/home-assistant/core/blob/dev/homeassistant/components/homematic/cover.py which is: if not (position := self._data.get("LEVEL_2", 0)): return None

I think this is the reason why also with value 0 the tilt position is not available any more. But position 0 is a valid position for tilt position closed.

Is my assumption right and do you agree that this check against 0 value for the tilt position should be removed (and only the previous check againt none or '' be kept)?

Thank you!

ivo7B4 avatar Feb 13 '22 15:02 ivo7B4

Yes my initial PR looked differently, but it was requested to change the code, which I did without thinking much about it. Here's the PR: https://github.com/home-assistant/core/pull/64208

This is how it initially should have looked. Feel free to contribute this again, along with an explanation why the optimized version that got merged is problematic. 👍

danielperna84 avatar Feb 13 '22 17:02 danielperna84

Hi @danielperna84, thank you for your information. I just wanted to add my explanaztion why the optimizied version is problematic to the pull request. I've thus opened the link you sent above but I get this message: "This conversation has been locked and limited to collaborators." I'm sorry that I'm not more familiar yet how else to add my comment. I'm fine to explain also to someone else why this is problematic. Thank you!

ivo7B4 avatar Feb 14 '22 15:02 ivo7B4

Hi @ivo7B4, does home-assistant/core#76507 match the problem you meant above (the tilt position of "Jalousien")?

juls avatar Aug 09 '22 09:08 juls

Hi @ivo7B4, does home-assistant/core#76507 match the problem you meant above (the tilt position of "Jalousien")?

Hi @juls This looks similar, I agree. I also liked the version of the PR clearly better. But it's still an improvement overall to before (before the PR) as the tilt position is still available if the jalousie are not fully open or closed and before, "Rolladenmodus" was not supported at all and works now. E.g. if you control sundlinds with these devices, you need the Rollandemodus as sundblinds don't have any tilt position. These are supported now (and did not work at all before). From my point of view, it would be best to: "Is my assumption right and do you agree that this check against 0 value for the tilt position should be removed (and only the previous check againt none or '' be kept)?"

ivo7B4 avatar Aug 09 '22 19:08 ivo7B4

Hi @ivo7B4, unfortunately I'm not deep enough into the codebase to evaluate the side effects, especially for the HmIPW-DRBL4 what this issue is about. But would you mind to create a new PR that could fix it (including that '0' check), as @danielperna84 also mentioned here? I guess a concrete fix could speed up the discussion and eventually get merged.

juls avatar Aug 10 '22 10:08 juls

Hi @juls I'm also nont in too much details yet. I was able however to install the development environment, tested the proposed solution as @danielperna84 proposed in this message: https://github.com/danielperna84/pyhomematic/issues/455#issuecomment-1007964881 I've tested this with both modes "Rollade" and "Jalousie" and both of them work perfectly fine as far as I can see, as I wrote here: https://github.com/danielperna84/pyhomematic/issues/455#issuecomment-1013907635

This was also the proposed solution for the pull request (PR) of @danielperna84 . Unfortunately, this got then further modifed as @danielperna84 explained in this message: https://github.com/danielperna84/pyhomematic/issues/455#issuecomment-1038279738 This as a reply to also my observation that the tilt value is removed when the position is 0, as I wrote here: https://github.com/danielperna84/pyhomematic/issues/455#issuecomment-1038209304

I think it is still much better as previously as it still works for both modes and as the tilt is available when the position is not 0 (where a tilt is also not really possible). But I also would clearly prefer the solution as was initially in the PR: From my point of view, the initial PR was the proper fix.

ivo7B4 avatar Aug 10 '22 20:08 ivo7B4