Add "Auto" HVAC Mode #44
Conversation
for more information, see https://pre-commit.ci
📝 WalkthroughWalkthroughAdds AUTO HVAC mode to BedJet climate control with a temperature-to-mode converter, dynamic device vs. configured temperature bounds and validation, automatic mode switching during AUTO temperature requests, and a new operating_mode sensor plus translation. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@custom_components/bedjet/climate.py`:
- Around line 300-311: The debug log in custom_components/bedjet/climate.py is
using %d which truncates float temperatures; update the _LOGGER.debug call (the
one referencing temp, self.temperature_unit,
self.hass.config.units.temperature_unit, self.hvac_mode, self._min_temp_actual,
self._max_temp_actual) to use %s or a float format like %.1f for temperature
values so decimal precision is preserved in the log message.
- Around line 216-225: When HVACMode.AUTO is chosen, the code silently falls
back to OperatingMode.COOL if current temp is None and also unconditionally sets
self._attr_hvac_mode to the requested AUTO even if the device command fails;
update the logic in the handler that calls temperature_to_mode and
self._device.set_operating_mode to (1) detect when cur_temp is None and emit a
warning via the integration logger describing that COOL is being used as a
fallback (reference temperature_to_mode, OperatingMode.COOL, and cur_temp), and
(2) only update self._attr_hvac_mode to hvac_mode (AUTO) after confirming the
call to self._device.set_operating_mode succeeded (or instead rely on the
coordinator/device update to set the attribute) so the UI state reflects the
real device state.
- Around line 148-170: The AUTO-mode temp bounds logic can ratchet because
self.max_temp/self.min_temp read the attributes being set; remove those from the
max()/min() calls so AUTO uses only the device-reported and hardcoded BedJet
extremes. In the update method where _attr_max_temp/_attr_min_temp are set for
HVACMode.AUTO (symbols: _attr_max_temp, _attr_min_temp, _bedjet_max_temp,
_bedjet_min_temp, state.maximum_temperature, state.minimum_temperature), change
the expressions to use max(state.maximum_temperature, self._bedjet_max_temp) and
min(state.minimum_temperature, self._bedjet_min_temp) respectively, leaving the
non-AUTO branch unchanged.
- Around line 263-323: async_set_temperature currently uses temp only inside the
AUTO branch but calls await self._device.set_temperature(temp) unconditionally,
causing NameError and potential None being passed into temperature_to_mode; fix
by reading temp = kwargs.get(ATTR_TEMPERATURE) at the start of
async_set_temperature, validate that temp is not None (raise
ServiceValidationError if missing), then proceed to handle ATTR_HVAC_MODE and
AUTO-specific logic using that temp (call temperature_to_mode(temp) only when
temp is present), and finally call await self._device.set_temperature(temp);
reference symbols: async_set_temperature, ATTR_TEMPERATURE, ATTR_HVAC_MODE,
HVACMode.AUTO, temperature_to_mode, ServiceValidationError, and
_device.set_temperature.
🧹 Nitpick comments (3)
custom_components/bedjet/sensor.py (1)
99-106: Consider usingSensorDeviceClass.ENUMwith explicitoptionsfor consistency.The
notificationsensor (line 48-67) usesSensorDeviceClass.ENUMwith a definedoptionslist and translated states. Theoperating_modesensor returns a free-form.title()string instead. This means state values won't be translatable and won't be constrained to known values in the HA UI.Also, if
device.state.operating_modecan ever beNone, this lambda will raise anAttributeError. Thenotificationsensor explicitly guards againstNone— consider doing the same here.custom_components/bedjet/climate.py (2)
86-94: Missing return type annotation and no docstring.
temperature_to_modeis a module-level public function but lacks a return type annotation and docstring. The thresholds (78°F, 87°F) are magic numbers that would benefit from named constants or at least inline documentation explaining their origin.♻️ Suggested improvement
-def temperature_to_mode(temperature: float): - as_fahrenheit = TemperatureConverter.convert( +def temperature_to_mode(temperature: float) -> OperatingMode: + """Derive the appropriate BedJet operating mode from a target temperature (Celsius).""" + as_fahrenheit = TemperatureConverter.convert( temperature, UnitOfTemperature.CELSIUS, UnitOfTemperature.FAHRENHEIT )
273-273: Use lazy%formatting in logger calls instead of f-strings.Home Assistant and Python logging best practices prefer
_LOGGER.debug("message %s", var)over f-strings to avoid string interpolation when the log level is disabled.
|
|
||
| # Only update hass-displayed mode if: | ||
| if ( | ||
| getattr(self, "_attr_hvac_mode", None) != HVACMode.AUTO # Not in auto mode | ||
| or state.operating_mode == OperatingMode.STANDBY # Bedjet turned itself off | ||
| ): | ||
| self._attr_hvac_mode = OPERATING_MODE_MAP[state.operating_mode] | ||
|
|
||
| self._max_temp_actual = state.maximum_temperature | ||
| self._min_temp_actual = state.minimum_temperature | ||
|
|
||
| if self._attr_hvac_mode == HVACMode.AUTO: | ||
| # Set min/max temp to the full range of bedjet temps to allow HA call to set_temperature to also set hvac state. | ||
| # Per-mode temp ranges validated manually below | ||
| self._attr_max_temp = max( | ||
| state.maximum_temperature, self._bedjet_max_temp, self.max_temp | ||
| ) | ||
| self._attr_min_temp = min( | ||
| state.minimum_temperature, self._bedjet_min_temp, self.min_temp | ||
| ) | ||
| else: | ||
| self._attr_max_temp = self._max_temp_actual | ||
| self._attr_min_temp = self._min_temp_actual |
There was a problem hiding this comment.
Potential stale bounds after AUTO mode switch.
In AUTO mode (lines 159-167), _attr_max_temp and _attr_min_temp are set to the broader of the device-reported bounds, the hardcoded BedJet bounds, and self.max_temp/self.min_temp. Since self.max_temp reads _attr_max_temp, this creates a ratchet effect — once the attribute grows to the BedJet max, max(state.maximum_temperature, self._bedjet_max_temp, self.max_temp) will always return at least the previous max, even if the intent was to narrow it.
Consider removing self.max_temp / self.min_temp from the max()/min() calls since the BedJet manual bounds already represent the widest range:
🔧 Suggested fix
- self._attr_max_temp = max(
- state.maximum_temperature, self._bedjet_max_temp, self.max_temp
- )
- self._attr_min_temp = min(
- state.minimum_temperature, self._bedjet_min_temp, self.min_temp
- )
+ self._attr_max_temp = max(
+ state.maximum_temperature, self._bedjet_max_temp
+ )
+ self._attr_min_temp = min(
+ state.minimum_temperature, self._bedjet_min_temp
+ )🤖 Prompt for AI Agents
In `@custom_components/bedjet/climate.py` around lines 148 - 170, The AUTO-mode
temp bounds logic can ratchet because self.max_temp/self.min_temp read the
attributes being set; remove those from the max()/min() calls so AUTO uses only
the device-reported and hardcoded BedJet extremes. In the update method where
_attr_max_temp/_attr_min_temp are set for HVACMode.AUTO (symbols:
_attr_max_temp, _attr_min_temp, _bedjet_max_temp, _bedjet_min_temp,
state.maximum_temperature, state.minimum_temperature), change the expressions to
use max(state.maximum_temperature, self._bedjet_max_temp) and
min(state.minimum_temperature, self._bedjet_min_temp) respectively, leaving the
non-AUTO branch unchanged.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@custom_components/bedjet/climate.py`:
- Around line 86-94: Add an explicit return type and a None-check to
temperature_to_mode: change the signature to include "-> OperatingMode" and at
the top of the function guard against None (e.g. if temperature is None: raise
ValueError("temperature must be provided")), then proceed to call
TemperatureConverter.convert with the validated float and return the appropriate
OperatingMode; reference temperature_to_mode, TemperatureConverter.convert and
OperatingMode when making the edits so the function is updated consistently with
its callers (e.g. async_set_temperature).
- Around line 116-117: Initialize the temperature actual bounds to the BedJet
manual bounds instead of 0.0 so range checks in async_set_temperature don't
falsely fail before the first coordinator update; replace the initializations of
_max_temp_actual and _min_temp_actual with the manual upper/lower bounds used by
this integration (e.g., use existing constants or properties such as
MANUAL_MAX_TEMP / MANUAL_MIN_TEMP or self._manual_max_temp /
self._manual_min_temp) so the comparison in async_set_temperature (the temp >
self._max_temp_actual check) behaves correctly until the coordinator populates
real values.
🧹 Nitpick comments (1)
custom_components/bedjet/climate.py (1)
272-274: Use lazy%-style formatting in logger calls instead of f-strings.f-strings are evaluated eagerly regardless of log level. HA's coding style and Python logging best practice prefer
%-style placeholders.- _LOGGER.debug( - f"Call to set temperature includes update to bedjet mode: {new_mode}" - ) + _LOGGER.debug( + "Call to set temperature includes update to bedjet mode: %s", + new_mode, + )
| def temperature_to_mode(temperature: float): | ||
| as_fahrenheit = TemperatureConverter.convert( | ||
| temperature, UnitOfTemperature.CELSIUS, UnitOfTemperature.FAHRENHEIT | ||
| ) | ||
| if as_fahrenheit <= 78: | ||
| return OperatingMode.COOL | ||
| if as_fahrenheit <= 87: | ||
| return OperatingMode.DRY | ||
| return OperatingMode.HEAT |
There was a problem hiding this comment.
Missing return type annotation and no input validation.
The function lacks a return type hint (-> OperatingMode) and doesn't guard against None input. Since it's called from async_set_temperature where temp originates from kwargs.get(ATTR_TEMPERATURE) (which can be None), a None value will cause a TypeError inside TemperatureConverter.convert.
Suggested fix
-def temperature_to_mode(temperature: float):
+def temperature_to_mode(temperature: float) -> OperatingMode:
+ if temperature is None:
+ raise ValueError("Temperature must not be None")
as_fahrenheit = TemperatureConverter.convert(
temperature, UnitOfTemperature.CELSIUS, UnitOfTemperature.FAHRENHEIT
)The None guard here is a safety net — ideally the caller should validate before invoking (see comment on async_set_temperature).
🤖 Prompt for AI Agents
In `@custom_components/bedjet/climate.py` around lines 86 - 94, Add an explicit
return type and a None-check to temperature_to_mode: change the signature to
include "-> OperatingMode" and at the top of the function guard against None
(e.g. if temperature is None: raise ValueError("temperature must be provided")),
then proceed to call TemperatureConverter.convert with the validated float and
return the appropriate OperatingMode; reference temperature_to_mode,
TemperatureConverter.convert and OperatingMode when making the edits so the
function is updated consistently with its callers (e.g. async_set_temperature).
| self._max_temp_actual = 0.0 | ||
| self._min_temp_actual = 0.0 |
There was a problem hiding this comment.
Initial _max_temp_actual = 0.0 will cause false validation failures if used before first update.
Both _max_temp_actual and _min_temp_actual start at 0.0. If async_set_temperature is called in AUTO mode before the coordinator delivers the first state update, the range check on line 313 (temp > self._max_temp_actual) will reject every valid temperature. Consider initializing these to the BedJet manual bounds instead.
Suggested fix
- self._max_temp_actual = 0.0
- self._min_temp_actual = 0.0
-
- self._bedjet_max_temp = (
+ self._bedjet_max_temp = (
TemperatureConverter.convert( # maximum per bedjet 3 manual
109, UnitOfTemperature.FAHRENHEIT, self.temperature_unit
)
)
- self._bedjet_min_temp = (
+ self._bedjet_min_temp = (
TemperatureConverter.convert( # minimum per bedjet 3 manual
66, UnitOfTemperature.FAHRENHEIT, self.temperature_unit
)
)
+ self._max_temp_actual = self._bedjet_max_temp
+ self._min_temp_actual = self._bedjet_min_temp🤖 Prompt for AI Agents
In `@custom_components/bedjet/climate.py` around lines 116 - 117, Initialize the
temperature actual bounds to the BedJet manual bounds instead of 0.0 so range
checks in async_set_temperature don't falsely fail before the first coordinator
update; replace the initializations of _max_temp_actual and _min_temp_actual
with the manual upper/lower bounds used by this integration (e.g., use existing
constants or properties such as MANUAL_MAX_TEMP / MANUAL_MIN_TEMP or
self._manual_max_temp / self._manual_min_temp) so the comparison in
async_set_temperature (the temp > self._max_temp_actual check) behaves correctly
until the coordinator populates real values.
| from homeassistant.exceptions import ServiceValidationError | ||
| from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback | ||
| from homeassistant.helpers.update_coordinator import DataUpdateCoordinator | ||
| from homeassistant.util.unit_conversion import TemperatureConverter |
There was a problem hiding this comment.
Rather than use TemperatureConverter to convert to Fahrenheit, just switch the comparison logic to use the appropriate Celsius value since all other temperatures are stored in Celsius.
| self._bedjet_max_temp = ( | ||
| TemperatureConverter.convert( # maximum per bedjet 3 manual | ||
| 109, UnitOfTemperature.FAHRENHEIT, self.temperature_unit | ||
| ) | ||
| ) | ||
| self._bedjet_min_temp = ( | ||
| TemperatureConverter.convert( # minimum per bedjet 3 manual | ||
| 66, UnitOfTemperature.FAHRENHEIT, self.temperature_unit | ||
| ) | ||
| ) |
There was a problem hiding this comment.
These numbers should probably be stored and retrieved from a constant in the pybedjet subfolder
| BedJetSensorEntityDescription( | ||
| key="operating_mode", | ||
| entity_category=EntityCategory.DIAGNOSTIC, | ||
| translation_key="operating_mode", | ||
| value_fn=lambda device: device.state.operating_mode.name.replace( | ||
| "_", " " | ||
| ).title(), | ||
| ), |
There was a problem hiding this comment.
Use an enum and translations for the operating mode value
| if as_fahrenheit <= 78: | ||
| return OperatingMode.COOL | ||
| if as_fahrenheit <= 87: | ||
| return OperatingMode.DRY | ||
| return OperatingMode.HEAT |
There was a problem hiding this comment.
This doesn't handle temperatures above 104F/Turbo mode
2 participants
This PR is an alternative to #24, allowing the integration to automatically map a temperature to an associated bedjet mode and enact that mode if necessary before setting the temperature. The intent of this feature is to allow me to more easily set a temperature when I wake up in the middle of the night w/o needing to remember which bedjet mode is most appropriate
It's an extension of #24 in the sense that the global temperature range is still calculated / set temps are validated manually, but now only when in "Auto" mode.
Bedjet-specific effective operating mode is now also included as a diagnostic sensor
Summary by CodeRabbit
New Features
Improvements