[breaking change] Combine supported fan modes & custom fan modes #15
Conversation
WalkthroughThe pull request introduces comprehensive changes to the Toshiba climate control system's fan mode handling. The modifications involve renaming and restructuring fan mode constants across multiple files, simplifying the fan level definitions, and updating the associated logic for processing fan modes. The changes aim to provide clearer semantics for fan levels and modes, removing redundant custom fan levels and introducing more standardized fan mode representations. Changes
Sequence DiagramsequenceDiagram
participant User
participant ClimateControl
participant ToshibaClimateUart
User->>ClimateControl: Set Fan Mode
ClimateControl->>ToshibaClimateUart: sendCmd(FanMode)
ToshibaClimateUart-->>ClimateControl: Confirm Mode Set
ClimateControl-->>User: Fan Mode Updated
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
components/toshiba_suzumi/toshiba_climate.cpp (1)
Clean up remaining legacy fan mode enum values
The fan mode implementation is mostly consistent, but there are still legacy enum values in the code:
components/toshiba_suzumi/toshiba_climate_mode.h: Remove unusedFANMODE_2andFANMODE_4enum values as they're not referenced in the implementation.The
FAN_AUTO = 65value and all other fan mode functionality is implemented correctly and consistently.🔗 Analysis chain
Line range hint
1-1: Verify all fan mode references in the codebase.Since this is a breaking change, let's verify that all fan mode references have been updated consistently.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining references to old fan mode names echo "Searching for old fan mode references..." rg -i "fanmode_[135]|auto.*=.*65|custom_fan_level_[13]" --type cpp # Search for potential fan mode usage in configuration files echo "Searching for fan mode usage in configuration files..." rg -i "fan_mode|fan_level" --type yamlLength of output: 384
Script:
#!/bin/bash # Get context around FAN_AUTO definition echo "Checking FAN_AUTO context..." rg -B 5 -A 5 "FAN_AUTO.*=.*65" components/toshiba_suzumi/toshiba_climate_mode.h # Search for fan mode related enums and function calls echo "Checking fan mode related code..." ast-grep --pattern 'enum $_ { $$$ FAN_$$$ $$$ }' echo "Checking fan mode function calls..." rg -B 2 -A 2 "set_fan_mode|get_fan_mode" --type cppLength of output: 4958
🧹 Nitpick comments (1)
components/toshiba_suzumi/toshiba_climate_mode.h (1)
31-37: Improve enum naming consistency.While the renaming of fan modes improves clarity, consider these suggestions:
- Rename
FANMODE_2toFAN_LOW_MEDIUMfor consistency with the string constant- Rename
FANMODE_4toFAN_MEDIUM_HIGHfor consistency with the string constantenum class FAN { FAN_QUIET = 49, FAN_LOW = 50, - FANMODE_2 = 51, + FAN_LOW_MEDIUM = 51, FAN_MEDIUM = 52, - FANMODE_4 = 53, + FAN_MEDIUM_HIGH = 53, FAN_HIGH = 54, FAN_AUTO = 65 };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
components/toshiba_suzumi/toshiba_climate.cpp(3 hunks)components/toshiba_suzumi/toshiba_climate_mode.cpp(1 hunks)components/toshiba_suzumi/toshiba_climate_mode.h(2 hunks)
🔇 Additional comments (4)
components/toshiba_suzumi/toshiba_climate_mode.cpp (1)
46-52: LGTM! Functions correctly handle the new fan mode structure.The implementation properly handles the conversion between string representations and fan modes, with appropriate error handling.
components/toshiba_suzumi/toshiba_climate.cpp (3)
229-243: LGTM! Clear and consistent fan mode handling in parseResponse.The implementation properly handles all fan modes with clear logging.
366-388: LGTM! Comprehensive fan mode control implementation.The control function properly handles all fan modes with appropriate logging and state management.
425-432: LGTM! Complete fan mode support in traits.The traits properly expose all available fan modes, both standard and custom.
| static const std::string &CUSTOM_FAN_LEVEL_2 = "Low-Medium"; | ||
| static const std::string &CUSTOM_FAN_LEVEL_4 = "Medium-High"; |
There was a problem hiding this comment.
Breaking change: Document fan level changes in README.
The removal of CUSTOM_FAN_LEVEL_1 and CUSTOM_FAN_LEVEL_3, and renaming of remaining levels improves clarity but breaks existing configurations.
Would you like me to help draft documentation explaining the migration path from old fan levels to new ones?
This PR allow the integration to have more "supported fan modes". This allow other integrations to use those fan modes.
For example versatile thermostat can use supported fan modes when the AC don't see the right temperature, to mix air.
I had some conflicts in the process, so I renamed some enum values.
WARNING ! It's a breaking change for people using fan speed in automations !
Summary by CodeRabbit
Release Notes
New Features
Improvements
Changes