-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support enocean teach in #76463
Support enocean teach in #76463
Conversation
- fixed variable access
- fixed variable access - fixed linter problems
- fixed variable access - fixed linter problems
Hey there @bdurrer, mind taking a look at this pull request as it has been labeled with an integration ( |
…r/core into supports_enocean_teach_in
flake8 fixes
This is awesome @flexxor 👍 the community was really looking for that one for sure. I hope this will get merged 🙏🙏🙏 |
@@ -0,0 +1,24 @@ | |||
teach_in_device: | |||
name: Teach-In Device | |||
description: Start the Teach-In-Process between the dongle and a device. During the teach-in normal packets are ignored |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does the teach in process do, and why is this a service? Is it something that the user wants to automate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Martin / Mr. Hjelmare,
the purpose of the process is to "pair" devices. Some devices need to be taught on which sender they have to listen to. As a user you do it once per device. The user pairs, for example a thermostat with the transceiver. After the pairing process was successful, the thermostat does not "throw" away telegrams from the taught-in sender.
You can do this with other software, too, of course.
The process is described in the technical specification: https://www.enocean-alliance.org/wp-content/uploads/2020/07/EnOcean-Equipment-Profiles-3-1.pdf, pages 17 and following
It's a service because the dongle (the usb transceiver / pi hat) (dongle.py) is not derived from an entity, but it is responsible for communication with the hardware devices.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. Then it doesn't sound like something that should be a service. Services are built for automation purposes.
I'd rather think this should be a flow that the user navigates from the frontend.
teach_in_response_packet.data[1:5] = packet.data[1:5] | ||
|
||
# set the bits of the byte for the success case (F0 = 11110000) | ||
teach_in_response_packet.data[4] = 0xF0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is protocol details that should be handled by a 3rd party library.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is, that the underlying library seems to be abandoned. I already made a pull-request there (as other people did). There is this github issue kipe/enocean#122 (Is your library still maintained).
I'm sorry that I had solved it that way. I saw other protocol specific code in the other devices, e.g. the light component, and thought it would be okay so that other home assistant users can do their pairing (teach-in) in Home Assistant with a few clicks and not manually build telegrams with the Dolphin Software (a windows software provided by the underlying hardware alliance which one can use to inspect and send telegrams)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have suggestions to circumvent this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some options:
- Try to reach out to the existing library author again via another media.
- Build another library.
return result | ||
|
||
while hex_value > 0: | ||
result.append(hex_value % 0x100) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Protocol details.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above :-(
channel = self.channel & 0xFF | ||
data = [RORG.VLD, 0x01] | ||
data.extend([channel]) | ||
data.extend([0x64]) # value to set: 0x64 = 100 = 100% |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Protocol details.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above :-(
import voluptuous as vol | ||
|
||
from homeassistant.components.enocean import services |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi @flexxor 👋
As this PR will most likely never be merged to the repo like all the others, I was implementing it in my custom integration available here, but when running it on HA, the import of this line does not work. I'm absolutely not familiar with HA code base and infra, so could you assist on that part ?
Here is the error:
File "/config/custom_components/enocean/__init__.py", line 6, in <module>
from homeassistant.components.enocean import services
ImportError: cannot import name 'services' from 'homeassistant.components.enocean' (/usr/src/homeassistant/homeassistant/components/enocean/__init__.py)
This will help a lot of people on french forums who have been waiting for this feature for many years. 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, in order to make it work, I had to change the import like so:
from homeassistant.components.enocean import services | |
from .services import async_setup_services |
async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: | ||
"""Set up the EnOcean component.""" | ||
|
||
if not hass.data.get(DOMAIN): | ||
services.async_setup_services(hass) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above suggestion:
services.async_setup_services(hass) | |
async_setup_services(hass) |
SERVICE_CALL_ATTR_TEACH_IN_SECONDS = "teach_in_time" | ||
SERVICE_CALL_ATTR_TEACH_IN_SECONDS_DEFAULT_VALUE_STR = "60" | ||
SERVICE_CALL_ATTR_TEACH_IN_SECONDS_DEFAULT_VALUE = 60 | ||
SERVICE_CALL_ATTR_TEACH_IN_BASE_ID_TO_USE = "teach_in_base_id" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this match services.yaml ?
SERVICE_CALL_ATTR_TEACH_IN_BASE_ID_TO_USE = "teach_in_base_id" | |
SERVICE_CALL_ATTR_TEACH_IN_BASE_ID_TO_USE = "base_id" |
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. |
There hasn't been any movement for some time here. I'll close now. We're trying to decrease our open PR buffer. Please open a new PR when ready to address the comments. Thanks! |
Pull request
Breaking change
Proposed change
Supports teach-in of 4BS and UTE telegrams. A new service has been introduced to start the teach-in process. During the runtime, response to teach-in request get sent, optionally with a specific base-id to use.
Type of change
The required library "kipe/enocean" is needed in version 0.60.1.
See https://github.com/kipe/enocean/releases/tag/0.60.1
Additional information
Checklist
black --fast homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
See documentation PR Update enocean.markdown - Documentation for teach-in support home-assistant.io#23852
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: