Gate demand response commands on dr_setting_use capability#117
Merged
Conversation
enable_demand_response/disable_demand_response were dispatched unconditionally even though the dr_setting_use capability flag exists on DeviceFeature. Every other controllable feature follows flag + _CAPABILITY_MAP entry + @requires_capability, so DR was the one gap. Add "dr_setting_use" to _CAPABILITY_MAP and decorate both methods. Fixes #114
Contributor
There was a problem hiding this comment.
Pull request overview
This PR closes the capability-gating gap for Demand Response (DR) controls by wiring the existing DeviceFeature.dr_setting_use flag into the capability checker and enforcing it via @requires_capability(...) on the MQTT control methods. This aligns DR with the established “feature flag + _CAPABILITY_MAP entry + decorator gate” pattern used by other controls.
Changes:
- Added
dr_setting_usetoMqttDeviceCapabilityChecker._CAPABILITY_MAP. - Gated
enable_demand_response/disable_demand_responsebehind@requires_capability("dr_setting_use"). - Added/updated tests to validate capability registration and gating behavior (Fixes #114).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/nwp500/device_capabilities.py | Registers dr_setting_use as a controllable capability so it can be checked centrally. |
| src/nwp500/mqtt/control.py | Applies @requires_capability("dr_setting_use") to DR enable/disable commands to prevent unsupported dispatch. |
| tests/test_device_capabilities.py | Extends capability-map tests to include dr_setting_use and updates expected control count. |
| tests/test_protocol_correctness.py | Adds protocol-level regression tests asserting DR commands are blocked when unsupported (and intended to be allowed when supported). |
Comment on lines
+331
to
+340
| @pytest.mark.asyncio | ||
| async def test_enable_allowed_when_supported(self, mock_device): | ||
| controller, publish = _make_controller() | ||
| controller._get_device_features = AsyncMock( | ||
| return_value=MagicMock(dr_setting_use=True) | ||
| ) | ||
|
|
||
| await controller.enable_demand_response(mock_device) | ||
| publish.assert_awaited_once() | ||
|
|
Owner
Author
There was a problem hiding this comment.
Fixed in ca1cf6e — added test_disable_allowed_when_supported so both commands' allowed-path are covered, not just enable.
Review feedback on #117 (Copilot): the PR description claimed both enable_demand_response and disable_demand_response were verified allowed-when-supported, but only enable_demand_response had that assertion. Add the matching test for disable_demand_response so a regression that gates one command but not the other would be caught.
Resolves conflicts with #115 (docstring fix) and #116 (freeze protection validation), both merged to main after this branch was cut. Conflicts were purely additive: dr_setting_use and freeze_protection_use both added to _CAPABILITY_MAP, and their respective test classes both added to test_protocol_correctness.py / test_device_capabilities.py. Combined capability count is now 10.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
enable_demand_response/disable_demand_response(src/nwp500/mqtt/control.py) were dispatched unconditionally, even though thedr_setting_usecapability flag exists onDeviceFeature(models/feature.py).feature.py+ entry in_CAPABILITY_MAP+@requires_capability(...)decorator (set_power→power_use,set_dhw_mode→dhw_use,configure_tou_schedule→program_reservation_use, etc). DR was the one gap."dr_setting_use": lambda f: bool(f.dr_setting_use)to_CAPABILITY_MAPand decorated both methods with@requires_capability("dr_setting_use").Fixes #114
Test plan
tests/test_device_capabilities.py: newtest_dr_setting_use_capability, updatedtest_get_available_controlsfor the new map entry (8 → 9 capabilities).tests/test_protocol_correctness.py: newTestDemandResponseCapabilityGateverifying both commands are blocked whendr_setting_useis falsy and allowed when truthy.608 passed, 4 skipped.