Skip to content
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

Add Bang & Olufsen device triggers #126755

Draft
wants to merge 10 commits into
base: dev
Choose a base branch
from

Conversation

mj23000
Copy link
Contributor

@mj23000 mj23000 commented Sep 25, 2024

Proposed change

Add device triggers for the physical "button" events that make sense and all triggers from the Beoremote One.

Most of the physical buttons have their own actions and do therefore not make sense to have device triggers for.

The Beoremote One has two menus that are available as device triggers: Light and Control.
These menus can be entered to trigger user renamable functions, for example: Control -> Window 1 which will trigger the Control/Func1_KeyPress and Control/Func1_KeyRelease device triggers. By pressing Select on the menus, the user can press most of the buttons on the remote like Control/Digit9_KeyRelease, Control/Blue_KeyPress, Control/Wind_KeyRelease etc.

The device triggers for the remote will only be exposed to the use if a Beoremote One is connected to the device.

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)
  • Deprecation (breaking change to happen in the future)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

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
  • I have followed the perfect PR recommendations
  • The code has been formatted using Ruff (ruff format 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.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

@emontnemery
Copy link
Contributor

emontnemery commented Sep 25, 2024

Please fix the merge conflicts

Comment on lines 112 to 132
def on_beo_remote_button_notification(self, notification: BeoRemoteButton) -> None:
"""Send beo_remote_button dispatch."""
# Trigger the device trigger
self.hass.bus.async_fire(
BANG_OLUFSEN_EVENT,
event_data={
CONF_TYPE: f"{notification.key}_{notification.type}",
CONF_DEVICE_ID: self._device.id,
},
)

def on_button_notification(self, notification: ButtonEvent) -> None:
"""Send button dispatch."""
# Trigger the device trigger
self.hass.bus.async_fire(
BANG_OLUFSEN_EVENT,
event_data={
CONF_TYPE: f"{notification.button}_{notification.state}",
CONF_DEVICE_ID: self._device.id,
},
)
Copy link
Contributor

@emontnemery emontnemery Sep 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest to, in an initial PR, add event entities, where each button is one event entity which can fire different events.
I'm not sure if device triggers are needed as well, maybe not?

Also, should the beo remotes be their own devices?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I could add the Beoremote One as a device, I'll take a look at that. I didn't even know event Entities were a thing, so I'll take a look at that as well. I'll convert this PR to a draft for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I've pushed an initial commit containing event entities for the physical buttons as well as Beoremote One keys and a device for the Beoremote One. There are 10 Event entities for the physical buttons and 58 for Beoremote One keys. This seems to be a bit excessive, but still seems to be a better way to handle events. What are your thoughts?

@mj23000 mj23000 marked this pull request as draft September 25, 2024 14:53
Comment on lines +28 to +29
"device_automation": {
"trigger_type": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you decide to still support device triggers, I'd suggest to have one type for each kind of keypress, and a subtype for each button. That should avoid overwhelming the user with a huge list of triggers.

Example:

"device_automation": {
"trigger_subtype": {
"button_1": "First button",
"button_2": "Second button",
"button_3": "Third button",
"button_4": "Fourth button",
"double_buttons_1_3": "First and Third buttons",
"double_buttons_2_4": "Second and Fourth buttons",
"dim_down": "Dim down",
"dim_up": "Dim up",
"turn_off": "[%key:common::action::turn_off%]",
"turn_on": "[%key:common::action::turn_on%]",
"1": "[%key:component::hue::device_automation::trigger_subtype::button_1%]",
"2": "[%key:component::hue::device_automation::trigger_subtype::button_2%]",
"3": "[%key:component::hue::device_automation::trigger_subtype::button_3%]",
"4": "[%key:component::hue::device_automation::trigger_subtype::button_4%]",
"clock_wise": "Rotation clockwise",
"counter_clock_wise": "Rotation counter-clockwise"
},
"trigger_type": {
"remote_button_long_release": "\"{subtype}\" released after long press",
"remote_button_short_press": "\"{subtype}\" pressed",
"remote_button_short_release": "\"{subtype}\" released",
"remote_double_button_long_press": "Both \"{subtype}\" released after long press",
"remote_double_button_short_press": "Both \"{subtype}\" released",
"initial_press": "\"{subtype}\" pressed initially",
"repeat": "\"{subtype}\" held down",
"short_release": "\"{subtype}\" released after short press",
"long_release": "[%key:component::hue::device_automation::trigger_type::remote_button_long_release%]",
"double_short_release": "[%key:component::hue::device_automation::trigger_type::remote_double_button_short_press%]",
"start": "[%key:component::hue::device_automation::trigger_type::initial_press%]"
}
},

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants