Skip to content

Conversation

@javicalle
Copy link
Contributor

Proposed change

Add unique_id property to the RFLink entities.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example entry for configuration.yaml:

There is no changes to device settings

# Example configuration.yaml entry
switch:
  - platform: rflink
    device_defaults:
      fire_event: true
      signal_repetitions: 2
    devices:
      newkaku_0000c6c2_1:
        name: Ceiling fan
      conrad_00785c_0a:
        name: Motion sensor kitchen

Additional information

This change has been successfully tested in my local environment, including automations, HA interactions and phisical devices interactions.

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

@frenck
Copy link
Member

frenck commented Apr 20, 2020

Hi there @javicalle!

Thanks for opening up a PR! However, there are all kinds of unrelated commits in this PR. Could you please clean that up so the PR only contains your changes?

Thanks! 👍

@javicalle
Copy link
Contributor Author

I think I was able to resolve the mess I had made with commits.
I hope the tests go fine

@javicalle
Copy link
Contributor Author

javicalle commented Apr 20, 2020

Local coverage ups to 100%
Not sure why codecov fails now. It is a problem?

@javicalle
Copy link
Contributor Author

I've been reviewing how the unique_id has been implemented in other integrations.
In general, they follow the recommendations of the documentation in this regard and those cases that are not, but there are possibilities, it has been rejected (#34668).

So, what is the situation in this case?
(disclaimer: the following is my personal analysis of RFLink library and may be incorrect and / or totally wrong)
The RFLink gateway reads RF signals. These signals have limited information on the device that generates them (besides the signal value) they have 'protocol', 'id' and 'button'. There are represented in the format <protocol> _ <id> _ <button> and is the value stored as _device_id

And, are these values uniques?
It cannot be guaranteed. In fact, some devices have a switch that allows you to change the ID to avoid conflicts, or there are others that change the ID every time you replace the batteries.
So it may not seem like a good candidate to define the unique_id value.

BUT IMHO there is another approach that is intended to follow this PR.
In the configuration you define an entity with a entity_id. And the RFLink library allows you to control changes to the device_id by defining device aliases. So you can have a device configured that is 'unique' in the configuration.
Obviously this value is "unique" in terms of the treatment the person gives it, not because it is identified by an internal value of the device that generates the signal. There is no possibility of accessing an identifier of type 'serial_number' or 'mac_address' from the RF signal.

But it is also true that this implementation enables the possibility of this operation being perverted so that you can substitute one physical device for another and assign it the same device_id.

So before going any further in this RP I think you should evaluate if the approach is correct. For my part, I don't think there is another way to generate a unique_id value for this type of integration.

bdraco and others added 23 commits May 12, 2020 10:03
Fix race condition that causes pairing and unpairing failures. ikalchev/HAP-python#246
Fix loop on dropped connections that causes temporary stalls and connection loss. ikalchev/HAP-python#249
Fix exception on missing video fields. ikalchev/HAP-python#245
When the websocket is created `SSLContext.load_default_certs` is called
each time which opens up the system default ssl certs file and reads it in

Normally this goes unnoticed, however since there are frequent connects
and disconnects of the websocket it was noticeable.
* add binary sensor platform to zwave_mqtt

* add tests for binary_sensor

* fix tests

* device class is required value

* Update homeassistant/components/zwave_mqtt/binary_sensor.py

use colon as separator

Co-authored-by: Martin Hjelmare <[email protected]>

* extend tests

* code optimize

* add test for enabling a legacy binary sensor

Co-authored-by: Martin Hjelmare <[email protected]>
…t#35545)

* Ensure homekit_controller recieves zeroconf c# updates

If an integration has a homekit config flow step
homekit controller would not see updates for
devices that were paired with it and would not
rescan for changes.

* Only send updates to homekit controller if the device is paired

This avoids the device showing up a second time.

* remove debug

* fix refactor error
…t#35555)

* New way to identify role, compatible with standalone controller or as part of unifios

* Remove error

* Bump dependency to v22

* Remove unused import
* Fix synology temperature rounding

* Remove stale comment
* Patch aiohttp client session close

* Add test

* Restore close regardless of auto_cleanup

* Close session instead of detaching and do not restore

* Delint test

* Add frame helper

* Use frame helper

* Test warning log when closing session

* Clean up

* Correct docstring

* Do not change shutdown

* Fix tests
…34670)

* Use helper functions for exception handling

* Create a separate class to handle communication with the device

* Update manifest

* Use coroutine for service setup

* Fix sensor update

* Update tests

* Fix MP1 switch

* Add device.py to .coveragerc

* Remove unnecessary blocking from test_learn_timeout

* Change access method for entries with default values

* Make the changes suggested by MartinHjelmare

* Remove dot from debug message

* Use underscore for unused variable
* Add integration for wiffi devices

wiffi devices are DIY board manufactured by stall.biz.
Several devices are available, e.g. a weather station (weatherman), an
indoor environmental sensor (wiffi-wz) and some more.
This intgration has been developed using a weatherman device, but should
also work for other devices from stall.biz.

* Fix pylint warning

* Use WIFFI / STALL WIFFI instead of wiffi to be consistent with stall.biz

* Don't update disabled entities.

* fix complains

- move wiffi specific code to pypi
- remove yaml configuration code

* incorporate various suggestions from code review

* fix remaining comments from Martin

* fix comments

* add tests for config flow

* fix comments

* add missing requirements for tests

* fix pylint warnings

* fix comments

* fix comments

remove debug log
rename .translations to translations

* rebase and adapt to latest dev branch

* Update homeassistant/components/wiffi/config_flow.py

Co-Authored-By: Martin Hjelmare <[email protected]>

* Update homeassistant/components/wiffi/config_flow.py

Co-Authored-By: Martin Hjelmare <[email protected]>

* fix missing import

Co-authored-by: Martin Hjelmare <[email protected]>
* Allow Plex discovery on demand

* Add new discovery source

* Add tests
* New forked_daapd component

* Bunch of changes

Add config flow and zeroconf
Add zones on callback when added by server
Add password auth
Add async_play_media for TTS
Add media_image_url
Add support for pipe control/input from librespot-java
Improve update callbacks

* Refactor as per code review suggestions

Move config_flow connection testing to pypi library (v0.1.4)
Remove use of ForkedDaapdData class
Decouple Master and Zone data and functions
Add updater class to manage websocket and entity updates

* More changes as per code review

Avoid direct access to entities in tests
Bump pypi version
Mark entities unavailable when websocket disconnected
Move config tests to test_config_flow
Move full url creation from media_image_url to library
Move updater entity from master to hass.data
Remove default unmute volume option
Remove name from config_flow
Remove storage of entities in hass.data
Use async_write_ha_state
Use signal to trigger update_options
Use unittest.mock instead of asynctest.mock

* Yet more changes as per code review

Add more assertions in tests
Avoid patching asyncio
Make off state require player state stopped
Only send update to existing zones
Split up some tests
Use events instead of async_block_till_done
Use sets instead of lists where applicable
Wait for pause callback before continuing TTS

* Remove unnecessary use of Future()

* Add pipes and playlists as sources

* Add support for multiple servers

Change config options to add max_playlists+remove use_pipe_control
Create Machine ID in test_connection and also get from zeroconf
Modify hass.data storage
Update host for known configurations
Use Machine ID in unique_ids, entity names, config title, signals

* Use entry_id as basis for multiple entries

* Use f-strings and str.format, abort for same host

* Clean up check for same host
Adminiuga and others added 24 commits May 23, 2020 06:22
…#35993)

* Centralite specific control seq of operation

* Remove Fan safeguards

* Split hvac_action property.

* Refactor hvac_action property.

Current hvac_action logic is Zen Within thermostat specific and differs
a bit from ZCL specs. Implement it as a separate class.

* Refactor hvac_action property for default thermostat

Follow more closely ZCL specs in parsing hvac state of the thermostat.
* Bump env_canada to 0.0.38

* Fix timestamp type
* Implement cover for "Shade" ZHA device type.
* Update ZHA cover tests.
* Add stop command
* Coverage.
* use Coerce(float) on service options

* defer the type from options list
@javicalle
Copy link
Contributor Author

It seems that I have done a mess with code rebase.
Also, there is no interest or activity about it, so I close the PR

@javicalle javicalle closed this May 24, 2020
@jeroen85
Copy link

It seems that I have done a mess with code rebase.
Also, there is no interest or activity about it, so I close the PR

That's a pity, that's exactly what I was looking for. Any chance this can still be implemented?

@qweluke
Copy link

qweluke commented Oct 22, 2021

@javicalle will you add unique_id? we really need this!

@MuadDib007
Copy link

unique_id would make my life much easier with regards to RFLink so I would like to see it as well.

@home-assistant home-assistant locked as resolved and limited conversation to collaborators Mar 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.