Skip to content
This repository has been archived by the owner on Feb 4, 2024. It is now read-only.

Pyhomematic/add/ipw key blind multi #318

Merged

Conversation

weissm
Copy link
Contributor

@weissm weissm commented Jul 30, 2020

Please point your pull request at the devel branch. Also provide some information about the changes.

This pull request:

  • fixes issue: #219
  • adds support for HomeMatic device: ccu3
  • does the following: The issue reported in #219 affects DRI-16 and DRI-32 and root caused as an vendor in here. In order to make these models work still, a work around was implemented by reading listdevices first before doing the actualy proxy.init call. To make this work, the newDevices methode needed some refinements.
  • There is still an issue pending, that the listDevices methode cannot return the full change set based on the reported finding: "One more detail I found while looking at this on RaspberryMatic with strace is, that the response of our server always gets cut to 8192 bytes." Hence, in this implementation only a limited list is returned.

pyhomematic/_hm.py Outdated Show resolved Hide resolved
pyhomematic/_hm.py Outdated Show resolved Hide resolved
pyhomematic/_hm.py Outdated Show resolved Hide resolved
pyhomematic/_hm.py Outdated Show resolved Hide resolved
@danielperna84 danielperna84 changed the base branch from master to devel August 2, 2020 14:06
@danielperna84
Copy link
Owner

I had a deeper look at this now and make some modifications. In my opinion the skipping isn't really necessary, except within newDevices. There I chose to look at the already known devices and skip adding the ones that are already present from a previous startup. I also didn't like the if statement that jumped out of the function. I believe it would have caused not being able to instantly detect devices that have just been paired to the CCU during runtime, requiring a complete restart of Home Assistant to work with a new device. The way it is now it just keeps working as before, but without creating duplicates during the second init we now will have for HmIP.

Could you please verify my code is working as intended?

@weissm
Copy link
Contributor Author

weissm commented Aug 3, 2020

Hi Daniel,

looks very good - thanks for taking the effort!
Just 2 questions from my side:

  • returning an empty list for for HMIP ports works. Just not clear if the general issue of the size limitation in the uplink packets might also exist in other ports and hence still lead to an error
  • I did not fully get the meaning of the variable. It seems to be set/unset, but never evaluated.

Again, thanks for taken care and spending the effort on keeping quality up!

Cheers, Matthias

@danielperna84
Copy link
Owner

  1. The issue only affects HomeMatic IP. The old RF and wired HomeMatic devices don't have the problem as they are controlled by a different server process that doesn't have the bug.
  2. Which variable are you referring to? hmip is set to True or False in line 302, depending on what the port of the remote is. Just like the result up in listDevices, but stored in the variable because it's being checked more often. That way multiple dictionary lookups can be avoided.

@weissm
Copy link
Contributor Author

weissm commented Aug 4, 2020

thanks for the prompt feedback.
(1) makes sense
(2) the actual var name global WORKING was hidden in the previous statement. Can you explain the var?

@danielperna84
Copy link
Owner

  1. I didn't pay attention to that. You pointed your PR to master instead of devel, resulting in a much bigger diff because devel already has code which is missing in master. I changed the target to devel, so now fewer changes are present in this PR, and none of them do anything with WORKING.

@danielperna84 danielperna84 merged commit 45ab6bc into danielperna84:devel Aug 4, 2020
danielperna84 added a commit that referenced this pull request Aug 4, 2020
* Add support for HmIPW-Devices (DRD3, SPI, DRBL4) (#310)

* Added support for HmIPW-DRD3, HmIPW-SPI and HmIPW-DRBL4

* Fix whitespace

Co-authored-by: Daniel Perna <danielperna84@gmail.com>

* support rx_mode (#313)

* support rx_mode

* fix whitespace

* feat: added support for HB-UNI-RGB-LED-CTRL (https://github.com/jp112sdl/HB-UNI-RGB-LED-CTRL) (#315)

Co-authored-by: Lukas Riegel <lukas.riegel@sap.com>

* Pyhomematic/add/ipw key blind multi (#318)

* Fixed work around for hm init issue

* added callback call

* remove comments

* removed pmatic

* fixed spacing

* according to alignment in #318, listdevice and interface if statement changed

* removed debug breakpoint setting

* added more generic check: proxy._remoteport == [2010, 32010, 42010]

* fixed proxy._remoteport in [2010, 32010, 42010] statement

* Limit to HmIP

* Remove whitespace

* Home Assistant doesn't use createDeviceObjects

* Change skipping mechanism

* Fix syntax

Co-authored-by: Daniel Perna <danielperna84@gmail.com>

* Add sensornode for HmIP Covers

* Update changelog

* Bump version

* Set https for json #311

Co-authored-by: P0L0 <1452110+p0l0@users.noreply.github.com>
Co-authored-by: Steffen Rusitschka <rusitschka@users.noreply.github.com>
Co-authored-by: lukasriegel <lukas-riegel@t-online.de>
Co-authored-by: Lukas Riegel <lukas.riegel@sap.com>
Co-authored-by: Matthias Weiss <matthias.h.weiss@arcor.de>
@weissm weissm deleted the pyhomematic/add/IPWKeyBlindMulti branch August 9, 2020 07:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants