-
-
Notifications
You must be signed in to change notification settings - Fork 35.8k
Convert flexit_bacnet fireplace mode to climate preset- Rename 'Boost… #155760
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
base: dev
Are you sure you want to change the base?
Convert flexit_bacnet fireplace mode to climate preset- Rename 'Boost… #155760
Conversation
…' preset to 'High' to match device terminology- Convert fireplace mode from switch entity to climate preset- Add translations and icons for all preset modes- Use operation_mode for accurate fireplace state detection
|
Hey there @lellky, @piotrbulinski, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
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.
Pull Request Overview
This PR refactors the Flexit BACnet integration to consolidate fireplace mode from a separate switch entity into a climate preset mode. The main changes involve migrating from using ventilation_mode to operation_mode for reading device state, adding fireplace as a climate preset, and updating preset names for better clarity.
Key changes:
- Replaces "boost" preset with "high" and adds "fireplace" as climate presets
- Removes fireplace mode switch entity and moves functionality to climate preset
- Changes climate entity to read state from
operation_modeinstead ofventilation_mode
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| homeassistant/components/flexit_bacnet/climate.py | Updates climate entity to use operation_mode for state, adds fireplace preset handling, renames boost to high |
| homeassistant/components/flexit_bacnet/const.py | Adds operation mode constants and mappings, replaces boost with high preset, removes ventilation-to-preset mapping |
| homeassistant/components/flexit_bacnet/switch.py | Removes fireplace mode switch entity definition |
| homeassistant/components/flexit_bacnet/strings.json | Adds climate preset translations, removes fireplace switch translation |
| homeassistant/components/flexit_bacnet/icons.json | Adds climate preset icons, removes fireplace switch icons |
| tests/components/flexit_bacnet/conftest.py | Adds operation_mode initialization to mock device |
| tests/components/flexit_bacnet/test_climate.py | Updates tests to set both ventilation_mode and operation_mode on mock |
| tests/components/flexit_bacnet/snapshots/test_climate.ambr | Updates snapshots for renamed presets and added translation key |
| tests/components/flexit_bacnet/snapshots/test_switch.ambr | Removes fireplace mode switch snapshots |
| PRESET_HIGH = "high" | ||
| PRESET_FIREPLACE = "fireplace" |
Copilot
AI
Nov 3, 2025
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.
These custom preset constants should include docstring comments explaining their purpose and when they're used, following Home Assistant's documentation standards for public constants.
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.
@copilot open a new pull request to apply changes based on this feedback
| Requires ClimateEntityFeature.PRESET_MODE. | ||
| """ | ||
| return VENTILATION_TO_PRESET_MODE_MAP[self.device.ventilation_mode] | ||
| return OPERATION_TO_PRESET_MODE_MAP.get(self.device.operation_mode, PRESET_HOME) |
Copilot
AI
Nov 3, 2025
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.
Using PRESET_HOME as a default fallback could mask issues when an unknown operation mode is encountered. Consider logging a warning when falling back to the default, or using PRESET_NONE as a more neutral fallback to indicate an unknown state.
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.
@copilot open a new pull request to apply changes based on this feedback
- Keep fireplace mode switch but disable it by default - Add deprecation warning when switch is used - Direct users to use climate preset mode instead - Provides smoother migration path for existing users
cd29eef to
8ebef4d
Compare
|
Good job! But, I have checked out your code and when running it I locally I found that it does not work properly. Changing preset to Fireplace using the card for the hvac entity works. But when trying to change back to any other preset does not work. Also I see no warning or repair issue risen when using the (now) deprecated switch. |
- Fix: Switching from fireplace preset to other presets now works correctly - When in fireplace mode, toggle it off before setting a new ventilation mode - trigger_fireplace_mode() acts as a toggle, so calling it while active turns it off - Fix: Deprecation warning now appears on integration load if fireplace switch is enabled - Previously only showed when switch was actively used - Now creates repair issue in async_setup_entry if switch is enabled - Add missing mock properties (fireplace_ventilation_status, cooker_hood_status) to conftest - Update snapshots to reflect correct fireplace switch state
Thanks for the feedback! 👍 I've tried addressing the issues you brought to light. I have tested with my device locally and changing away from Fireplace-mode now works in the card. |
I can verify that you latest change works; the repair issue is risen and switching presets works. Great! |
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.
Looks good to me!
Breaking change
The fireplace mode switch entity has been deprecated and disabled by default. It will be removed in a future version. Users with existing automations can continue using it, but will receive a repair notification prompting them to migrate to the climate preset.
For users with existing fireplace switch automations:
A repair issue will guide you to update your automations:
Before (deprecated):
After (recommended):
Additionally, the "Boost" preset has been renamed to "High" to match the device's terminology.
Proposed change
This PR converts the fireplace mode in the flexit_bacnet integration from a switch entity to a proper climate preset mode, matching how it appears on the physical Flexit device. It also renames the "Boost" preset to "High" to align with device terminology.
Problem: The fireplace mode was implemented as a switch entity, but on the physical Flexit device it functions as a ventilation mode just like "Home", "Away", and "High" (previously "Boost"). This inconsistency made it less intuitive for users and didn't match the device's actual interface.
Solution:
switch.flexit_nordic_fireplace_modeto a climate presetoperation_modeproperty to detect fireplace state (which correctly reportsOPERATION_MODE_FIREPLACEwhen active)Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
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.To help with the load of incoming pull requests: