Skip to content

Add new getters/setters for %RH and CO₂ sensor control, heating adjust mode, and defrost mode.#71

Merged
yozik04 merged 7 commits intoyozik04:masterfrom
karrenberg:dev
Feb 26, 2025
Merged

Add new getters/setters for %RH and CO₂ sensor control, heating adjust mode, and defrost mode.#71
yozik04 merged 7 commits intoyozik04:masterfrom
karrenberg:dev

Conversation

@karrenberg
Copy link
Contributor

Hi! First pull request for me, please help me get this right :).
Thank you for your work on the websocket API! It's really awesome to be able to hack on this myself instead of relying on proprietary stuff. I hope I can contribute back with this.

I tried to make this as clean as possible. Of course it makes much more sense with the Home Assistant side of things - I also have a commit for the HA Vallox component ready that will expose all these settables as switches/numbers/selects and also create more sensors for other data that is displayed in the Vallox web GUI. It only needs a bit of cleanup before it will go up for review, too. All settables have been tested in production to match what the web GUI does. There are a few more that I couldn't quite identify, e.g. some obscure defrost settings with offset/param pairs that I couldn't make sense of (value mismatches between Vallox GUI and API readouts).

  • New settables:

    • A_CYC_<HOME|AWAY|BOOST>_RH_CTRL_ENABLED
    • A_CYC_<HOME|AWAY|BOOST>_CO2_CTRL_ENABLED
    • A_CYC_CO2_THRESHOLD
    • A_CYC_RH_LEVEL_MODE
    • A_CYC_RH_BASIC_LEVEL
    • A_CYC_PARTIAL_BYPASS
    • A_CYC_SUPPLY_HEATING_ADJUST_MODE
    • A_CYC_POST_HEATER_WINTER_SETPOINT
    • A_CYC_COOLRECOVERY_DISABLED
    • A_CYC_DEFROST_MODE
    • A_CYC_SUPPLY_AIR_DEFROST_TEMP
  • These all correspond to options available in the Vallox web GUI.

  • New enums:

    • CellState
    • SupplyHeatingAdjustMode
    • DefrostMode
  • New helpers:

    • MetricData::supply_heating_adjust_mode()
    • MetricData::get_supply_heating_adjust_mode()
    • MetricData::get_supply_heating_adjust_mode_name()
    • MetricData::defrost_mode()
    • MetricData::get_defrost_mode()
    • MetricData::get_defrost_mode_name()
    • MetricData::get_rh_sensor_control()
    • MetricData::get_co2_sensor_control()
    • MetricData::get_rh_sensor_control_mode()
    • MetricData::get_rh_sensor_limit()
    • MetricData::get_co2_sensor_limit()
    • Vallox::set_rh_sensor_control()
    • Vallox::set_co2_sensor_control()
    • Vallox::set_rh_sensor_control_mode()
    • Vallox::set_rh_sensor_limit()
    • Vallox::set_co2_sensor_limit()
    • Vallox::set_supply_heating_adjust_mode()
    • Vallox::set_defrost_mode()

- New settables:
  - A_CYC_<HOME|AWAY|BOOST>_RH_CTRL_ENABLED
  - A_CYC_<HOME|AWAY|BOOST>_CO2_CTRL_ENABLED
  - A_CYC_CO2_THRESHOLD
  - A_CYC_RH_LEVEL_MODE
  - A_CYC_RH_BASIC_LEVEL
  - A_CYC_PARTIAL_BYPASS
  - A_CYC_SUPPLY_HEATING_ADJUST_MODE
  - A_CYC_POST_HEATER_WINTER_SETPOINT
  - A_CYC_COOLRECOVERY_DISABLED
  - A_CYC_DEFROST_MODE
  - A_CYC_SUPPLY_AIR_DEFROST_TEMP
- These all correspond to options available in the Vallox web GUI.

- New enums:
  - CellState
  - SupplyHeatingAdjustMode
  - DefrostMode

- New helpers:
  - MetricData::supply_heating_adjust_mode()
  - MetricData::get_supply_heating_adjust_mode()
  - MetricData::get_supply_heating_adjust_mode_name()
  - MetricData::defrost_mode()
  - MetricData::get_defrost_mode()
  - MetricData::get_defrost_mode_name()
  - MetricData::get_rh_sensor_control()
  - MetricData::get_co2_sensor_control()
  - MetricData::get_rh_sensor_control_mode()
  - MetricData::get_rh_sensor_limit()
  - MetricData::get_co2_sensor_limit()
  - Vallox::set_rh_sensor_control()
  - Vallox::set_co2_sensor_control()
  - Vallox::set_rh_sensor_control_mode()
  - Vallox::set_rh_sensor_limit()
  - Vallox::set_co2_sensor_limit()
  - Vallox::set_supply_heating_adjust_mode()
  - Vallox::set_defrost_mode()

Note: I also have a commit for the HA Vallox component ready that will expose all these as switches/numbers/selects and also create more sensors for other data that is displayed in the Vallox web GUI.
@codecov-commenter
Copy link

codecov-commenter commented Feb 22, 2025

Codecov Report

Attention: Patch coverage is 95.40230% with 4 lines in your changes missing coverage. Please review.

Project coverage is 88.81%. Comparing base (c8491e1) to head (a79582f).
Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
vallox_websocket_api/vallox.py 95.29% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #71      +/-   ##
==========================================
+ Coverage   87.88%   88.81%   +0.93%     
==========================================
  Files           7        7              
  Lines         553      635      +82     
==========================================
+ Hits          486      564      +78     
- Misses         67       71       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@yozik04
Copy link
Owner

yozik04 commented Feb 22, 2025

Hi @karrenberg, I am glad to see your interest in improving this project. I am happy to help. Did the review. Take a look.

@karrenberg
Copy link
Contributor Author

Hi @karrenberg, I am glad to see your interest in improving this project. I am happy to help. Did the review. Take a look.

Thanks for the review @yozik04, much appreciated! I'll make the changes as soon as I find the time.

@karrenberg karrenberg requested a review from yozik04 February 25, 2025 22:25
@property
def rh_sensor_control_mode(self) -> Optional[int]:
"""Return the RH sensor control mode (0 for automatic, 1 for manual)"""
return self.get(SET_RH_SENSOR_CONTROL_MODE)
Copy link
Owner

Choose a reason for hiding this comment

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

The last thing that bothers me. Do we want to use enum here or we will make it boolean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yeah, good point. I don't have a preference, but int is clearly not a good choice :).

Copy link
Owner

Choose a reason for hiding this comment

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

Ok. I changed the name and made it boolean

@yozik04 yozik04 merged commit 6be31ec into yozik04:master Feb 26, 2025
@yozik04
Copy link
Owner

yozik04 commented Feb 26, 2025

@karrenberg Thank you for the effort. Really appreciated.

@yozik04 yozik04 changed the title Add various new settables incl some getters/setters and enums. Add new getters/setters for %RH and CO₂ sensor control, heating adjust mode, and defrost mode. Feb 26, 2025
@yozik04
Copy link
Owner

yozik04 commented Feb 26, 2025

BTW: CellState is not used at the moment. Did you had any plans for it?

@karrenberg
Copy link
Contributor Author

karrenberg commented Feb 26, 2025

BTW: CellState is not used at the moment. Did you had any plans for it?

It's a refactor to get rid of the hardcoded values in the Vallox HA component:

VALLOX_CELL_STATE_TO_STR = {
   0: "Heat Recovery",
   1: "Cool Recovery",
   2: "Bypass",
   3: "Defrosting",

I believe that ideally all hardcoded values should be abstracted by the websocket API. It's a bit overkill for all the A_CYC_ constants, but things like these integer values that correspond to specific settings should be kept behind the API interface.

@yozik04
Copy link
Owner

yozik04 commented Feb 26, 2025

But let's make a property that will return CellState

@karrenberg
Copy link
Contributor Author

karrenberg commented Feb 26, 2025

@karrenberg Thank you for the effort. Really appreciated.

Thanks a lot for the quick responses, the detailed review and your fixes! Very much looking forward to merging the HA changes to make use of the new stuff :).

Sneak peek without any GUI efforts (ignore the weird defrost sensors, I was trying to find some more of the settings exposed in Vallox' web GUI):
[image redacted]

@yozik04
Copy link
Owner

yozik04 commented Feb 26, 2025

Nice. I have released 5.4.0.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants