`ramses_cc.set_zone_mode` - broken?

iMiMx commented 9 months ago

hallway_heating_boost_1hour:
  sequence:
    - service: ramses_cc.set_zone_mode
      data:
        entity_id: climate.hallway
        setpoint: '{{ states.input_number.hallway_heating_boost_temperature.state | float }}'
        duration: {minutes: 60}

The above worked on 21.40, in 31.7 it throws an error:

Logger: homeassistant.components.script.hallway_heating_boost_1hour
Source: helpers/script.py:1805
Integration: Scripts (documentation, issues)
First occurred: 07:30:57 (1 occurrences)
Last logged: 07:30:57

hallway_heating_boost_1hour: Error executing script. Invalid data for call_service at pos 1: extra keys not allowed @ data['setpoint']

Checking with Developer Tools -> Services it seems that the below, 'mode' is apparently no longer optional?

service: ramses_cc.set_zone_mode
data:
  entity_id: climate.hallway
  setpoint: 21
  duration: {minutes: 60}

"This service requires field mode, which must be provided under data:"

mode The permanency of the override. Optional, one of: follow_schedule, advanced_override (until next scheduled setpoint), temporary_override (see: duration and until), or permanent_override (indefinitely).

With 'mode' set it then throws another error:

service: ramses_cc.set_zone_mode
data:
  entity_id: climate.hallway
  setpoint: 21
  mode: temporary_override
  duration: {minutes: 60}

This service requires field until, which must be provided under data:

But this should be 'mutually exlusive' with duration?

until
The end of the temporary_override. Mutually exclusive with duration.

bluediamond1984 commented 9 months ago

It seems a bug in services.yaml. Not every zone_mode accepts all fields, like follow_schedule isn't allowed to have any of them. But duration and until are declared as required. To fix the issue change the required state to false on line 290 en 299.

raldred commented 9 months ago

I get a totally different error

Failed to call service ramses_cc/set_zone_mode. value must be one of ['follow_schedule'] for dictionary value @ data['mode']

zxdavb commented 9 months ago

related to #169

zxdavb commented 9 months ago

This is teh current test suite, that is passing:

TESTS_SET_ZONE_MODE: dict[str, dict[str, Any]] = {
    "00": {"mode": "follow_schedule"},
    "01": {"mode": "permanent_override", "setpoint": 18.5},
    "02": {"mode": "advanced_override", "setpoint": 19.5},
    "03": {"mode": "temporary_override", "setpoint": 20.5, "duration": {"minutes": 90}},
    "04": {"mode": "temporary_override", "setpoint": 21.5, "duration": {"hours": 3}},
}  # need to add until...

... if there remains an issue, it is in ramses_rf.

zxdavb commented 9 months ago

Please re-open this issue if the bug persists after release 0.41.8 (there are two tracks: 0.31.7 and 0.41.7 - 0.31.x will no longer be maintained).

iMiMx commented 8 months ago

Upgraded to latest stable (0.31.7.) seems 'mode' which is apparently optional, is not optional.

service: ramses_cc.set_zone_mode
data:
  entity_id: climate.living_room
  setpoint: 22
  duration: {minutes: 60}

This service requires field mode, which must be provided under data:

service: ramses_cc.set_zone_mode
data:
  entity_id: climate.living_room
  setpoint: 22
  duration: {minutes: 60}     
  mode: temporary_override

This service requires field until, which must be provided under data:

No change from the reported issue, with 0.31.7. Downgrading again to 0.21.4, last 'stable' for me.

zxdavb commented 8 months ago

Upgraded to latest stable (0.31.7) seems 'mode' which is apparently optional, is not optional.

Version 0.31.11 has been released and it should address this issue. Please let us know if it does not.

No change from the reported issue, with 0.31.7. Downgrading again to 0.21.4, last 'stable' for me.

This is a known bug in 0.31.7.

To be clear - 0.21.4, 0.31.7 and 0.31.11 are all on the 'stable' track, regardless of of being bug-free, or not.

iMiMx commented 8 months ago

0.31.11 - still same issue. Downgrading to 0.21.4, where the below works:

service: ramses_cc.set_zone_mode
data:
  entity_id: climate.living_room
  setpoint: 22
  duration:
    minutes: 60

Mode should be optional (it's currently not) and until should be mutually exclusive with duration (it's currently not)

zxdavb commented 8 months ago

This is the call I'm using in test/dev:

service: ramses_cc.set_zone_mode
data:
  entity_id: climate.01_123456_02
  setpoint: 21
  duration: {minutes: 60}

... it throws an error: This service requires field mode, which must be provided under data:

But to me, this is obvious that it is mode: temporary_override.

If we add mode:, we get:

service: ramses_cc.set_zone_mode
data:
  entity_id: climate.01_123456_02
  setpoint: 21
  mode: temporary_override 
  duration: {minutes: 60}

... but, it throws an error: This service requires field until, which must be provided under data:

If we use until:, we get:

service: ramses_cc.set_zone_mode
data:
  entity_id: climate.01_123456_02
  setpoint: 21
  mode: temporary_override 
  until: "2024-03-16 14:00:00"

... but, it throws an error: This service requires field duration, which must be provided under data:

If we use duration: and until:, we get:

service: ramses_cc.set_zone_mode
data:
  entity_id: climate.01_123456_02
  setpoint: 21
  mode: temporary_override 
  duration: {minutes: 60}  
  until: "2024-03-16 14:00:00"

... but, it throws an error: Failed to call service ramses_cc.set_zone_mode. extra keys not allowed @ data['duration']. Got None extra keys not allowed @ data['setpoint']. Got None extra keys not allowed @ data['until']. Got None value must be one of [<ZoneMode.SCHEDULE: 'follow_schedule'>] for dictionary value @ data['mode']. Got None

This message should use a str rather than a strEnum.

iMiMx commented 8 months ago

Documentation states 'optional'

mode
The permanency of the override. Optional, one of: follow_schedule, advanced_override (until next scheduled setpoint), temporary_override (see: duration and until), or permanent_override (indefinitely).

You add 'mode' it then throws an error about 'until' even with duration set:

duration
The duration of the temporary_override. Mutually exclusive with until.   {"minutes": 135} 

until
The end of the temporary_override. Mutually exclusive with duration.

zxdavb commented 8 months ago

Ok, this is the test suite that I'll attempt to get working:

TESTS_SET_ZONE_MODE_GOOD: dict[str, dict[str, Any]] = {
    "11": {"mode": "follow_schedule"},
    "21": {"mode": "permanent_override", "setpoint": 12.1},
    "31": {"mode": "advanced_override", "setpoint": 13.1},
    "41": {"mode": "temporary_override", "setpoint": 14.1},  # default duration 1 hour
    "52": {"mode": "temporary_override", "setpoint": 15.1, "duration": {"hours": 5}},
    "62": {"mode": "temporary_override", "setpoint": 16.1, "until": _UNTIL},
}
TESTS_SET_ZONE_MODE_FAIL: dict[str, dict[str, Any]] = {
    "00": {},  # #                                                     missing mode
    "12": {"mode": "follow_schedule", "setpoint": 11.2},  # #          *extra* setpoint
    "20": {"mode": "permanent_override"},  # #                         missing setpoint
    "22": {"mode": "permanent_override", "setpoint": 12.2, "duration": {"hours": 5}},
    "23": {"mode": "permanent_override", "setpoint": 12.3, "until": _UNTIL},
    "29": {"setpoint": 12.9},  # #                                     missing mode
    "30": {"mode": "advanced_override"},  # #                          missing setpoint
    "32": {"mode": "advanced_override", "setpoint": 13.2, "duration": {"hours": 5}},
    "33": {"mode": "advanced_override", "setpoint": 13.3, "until": _UNTIL},
    "40": {"mode": "temporary_override"},  # #                         missing setpoint
    "50": {"mode": "temporary_override", "duration": {"hours": 5}},  # missing setpoint
    "59": {"setpoint": 15.9, "duration": {"hours": 5}},  # #           missing mode
    "60": {"mode": "temporary_override", "until": _UNTIL},  # #        missing setpoint
    "69": {"setpoint": 16.9, "until": _UNTIL},  # #                    missing mode
    "79": {
        "mode": "temporary_override",
        "setpoint": 16.9,
        "duration": {"hours": 5},
        "until": _UNTIL
    },
}

zxdavb commented 8 months ago

ok, drama is that this is enforced in two places:

services.yaml, and
schemas.py (the voluptuous schemas)

The above code is only testing the latter

zxdavb commented 8 months ago

This service requires field mode, which must be provided under data:

I'm going to make mode non-optional. Thus, the above error message is acceptable.

My rationale is as follows. First, it is clear that the mode is:

follow_schedule if: no setpoint
temporary_override if: has duration or until

But what if it has a setpoint, and no duration/until? You could argue it would be until:

next scheduled setpoint -> advanced_override
indefinitely -> permanent_override
for an hour (like a DHW 'boost') -> temporary_override

Easiest way to resolve the argument, to remove all ambiguity, is to make mode required.

Similarly, setpoint is now a required value, if the mode is not follow_schedule.

I will be doing the same for the set_dhw_mode service call.

zxdavb commented 8 months ago

And for DHW, we have:

TESTS_SET_DHW_MODE_GOOD = {
    "11": {"mode": "follow_schedule"},
    "21": {"mode": "permanent_override", "active": True},
    "31": {"mode": "advanced_override", "active": True},
    "41": {"mode": "temporary_override", "active": True},  # default duration 1 hour
    "52": {"mode": "temporary_override", "active": True, "duration": {"hours": 5}},
    "62": {"mode": "temporary_override", "active": True, "until": _UNTIL},
}
TESTS_SET_DHW_MODE_FAIL: dict[str, dict[str, Any]] = {
    "00": {},  # #                                                     missing mode
    "12": {"mode": "follow_schedule", "active": True},  # #            *extra* active
    "20": {"mode": "permanent_override"},  # #                         missing active
    "22": {"mode": "permanent_override", "active": True, "duration": {"hours": 5}},
    "23": {"mode": "permanent_override", "active": True, "until": _UNTIL},
    "29": {"active": True},  # #                                       missing mode
    "30": {"mode": "advanced_override"},  # #                          missing active
    "32": {"mode": "advanced_override", "active": True, "duration": {"hours": 5}},
    "33": {"mode": "advanced_override", "active": True, "until": _UNTIL},
    "40": {"mode": "temporary_override"},  # #                         missing active
    "42": {"mode": "temporary_override", "active": False}, # #         missing duration
    "50": {"mode": "temporary_override", "duration": {"hours": 5}},  # missing active
    "59": {"active": True, "duration": {"hours": 5}},  # #             missing mode
    "60": {"mode": "temporary_override", "until": _UNTIL},  # #        missing active
    "69": {"active": True, "until": _UNTIL},  # #                      missing mode
    "79": {
        "mode": "temporary_override",
        "active": True,
        "duration": {"hours": 5},
        "until": _UNTIL
    },
}

NOTE: tests 41/42, and c.f. Zone's 41 - DHW's 41 is what is classically know as a DHW 'boost'

zxdavb commented 8 months ago

It is currently unclear to me how I test service.yaml (the developer page in the web UI).

zxdavb / ramses_cc

`ramses_cc.set_zone_mode` - broken? #163