Conversation
Qinghao's branch
There was a problem hiding this comment.
Pull request overview
This pull request adds support for additional Home Assistant device types (switch, cover, and fan) to the VOLTTRON Platform Driver, refactors the Home Assistant driver interface to use a cleaner dispatch table pattern, updates PyYAML dependency, and implements Python 3.12+ compatibility for argument parsing.
Key Changes:
- Refactored Home Assistant driver to use a dispatch table pattern for extensibility when handling different device domains
- Added write access support for switch, cover, and fan devices with corresponding API methods and handler functions
- Updated Python 3.12+ compatibility in argparse handling to accommodate changes in
_parse_optionalreturn values - Updated PyYAML from 6.0 to 6.0.1
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| volttron/platform/config.py | Adds conditional logic to handle Python 3.12+ changes in argparse's _parse_optional method return signature |
| requirements.py | Updates PyYAML dependency from 6.0 to 6.0.1 |
| services/core/PlatformDriverAgent/platform_driver/interfaces/home_assistant.py | Major refactoring: introduces WRITE_HANDLERS dispatch table, extracts device-specific logic into dedicated handler methods (_write_switch, _write_cover, _write_fan), and adds API methods for new device types |
| services/core/PlatformDriverAgent/tests/test_home_assistant.py | Adds three new test fixtures (config_store_switch, config_store_cover, config_store_fan) and six new test functions to validate switch, cover, and fan device operations |
| docs/source/agent-framework/driver-framework/home-assistant/HomeAssistantDriver.rst | Adds comprehensive documentation for the new switch, cover, and fan write access support with examples and usage instructions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Write Access Support for Switch, Cover, and Fan | ||
| =============================================== | ||
|
|
||
| Beginning in this version, the Home Assistant Driver supports *write access* for | ||
| three additional Home Assistant device domains: | ||
|
|
||
| - ``switch`` | ||
| - ``cover`` | ||
| - ``fan`` | ||
|
|
||
| This expands VOLTTRON's ability to control additional IoT devices by allowing | ||
| ``set_point`` operations to call the appropriate Home Assistant REST API | ||
| service endpoints. | ||
|
|
||
| Below is a summary of write-capable entity points and their expected values. | ||
|
|
||
| Switch Write Support | ||
| -------------------- | ||
|
|
||
| Switches support a single writable point: | ||
|
|
||
| ============ ========================== | ||
| Entity Point Description | ||
| ============ ========================== | ||
| state ``0 = off``, ``1 = on`` | ||
| ============ ========================== | ||
|
|
||
| Example registry entry: | ||
|
|
||
| .. code-block:: json | ||
|
|
||
| { | ||
| "Entity ID": "switch.my_switch", | ||
| "Entity Point": "state", | ||
| "Volttron Point Name": "switch_state", | ||
| "Units": "On/Off", | ||
| "Writable": true, | ||
| "Type": "int", | ||
| "Notes": "Example switch" | ||
| } | ||
|
|
||
| Calling: | ||
|
|
||
| .. code-block:: bash | ||
|
|
||
| vctl rpc call platform.driver set_point home_assistant switch_state 1 | ||
|
|
||
| invokes Home Assistant service: | ||
|
|
||
| ``switch.turn_on`` | ||
|
|
||
|
|
||
| Cover Write Support | ||
| -------------------- | ||
|
|
||
| Covers support multiple writable points, enabling not just open/close but also | ||
| position and tilt control. | ||
|
|
||
| ================= =============================================== | ||
| Entity Point Description | ||
| ================= =============================================== | ||
| state ``0=close``, ``1=open``, ``2=stop`` | ||
| position Integer from 0–100 (percentage open) | ||
| tilt Integer from 0–100 (tilt angle) | ||
| ================= =============================================== | ||
|
|
||
| Example registry entries: | ||
|
|
||
| .. code-block:: json | ||
|
|
||
| [ | ||
| { | ||
| "Entity ID": "cover.my_shade", | ||
| "Entity Point": "state", | ||
| "Volttron Point Name": "shade_state", | ||
| "Writable": true, | ||
| "Type": "int" | ||
| }, | ||
| { | ||
| "Entity ID": "cover.my_shade", | ||
| "Entity Point": "position", | ||
| "Volttron Point Name": "shade_position", | ||
| "Writable": true, | ||
| "Type": "int" | ||
| } | ||
| ] | ||
|
|
||
| ``set_point`` automatically maps to HA services: | ||
|
|
||
| - ``cover.open_cover`` | ||
| - ``cover.close_cover`` | ||
| - ``cover.stop_cover`` | ||
| - ``cover.set_cover_position`` | ||
|
|
||
|
|
||
| Fan Write Support | ||
| ----------------- | ||
|
|
||
| Fans support on/off, speed percentage, and direction control. | ||
|
|
||
| ================= =========================================== | ||
| Entity Point Description | ||
| ================= =========================================== | ||
| state ``0=off``, ``1=on`` | ||
| percentage Speed percentage (0–100) | ||
| direction ``0 = forward``, ``1 = reverse`` | ||
| ================= =========================================== | ||
|
|
||
| Example registry entries: | ||
|
|
||
| .. code-block:: json | ||
|
|
||
| [ | ||
| { | ||
| "Entity ID": "fan.living_room_fan", | ||
| "Entity Point": "state", | ||
| "Volttron Point Name": "fan_state", | ||
| "Writable": true, | ||
| "Type": "int" | ||
| }, | ||
| { | ||
| "Entity ID": "fan.living_room_fan", | ||
| "Entity Point": "percentage", | ||
| "Volttron Point Name": "fan_percentage", | ||
| "Writable": true, | ||
| "Type": "int" | ||
| } | ||
| ] | ||
|
|
||
| These map to Home Assistant services: | ||
|
|
||
| - ``fan.turn_on`` / ``fan.turn_off`` | ||
| - ``fan.set_percentage`` | ||
| - ``fan.set_direction`` | ||
|
|
||
| Example usage: | ||
|
|
||
| .. code-block:: bash | ||
|
|
||
| vctl rpc call platform.driver set_point home_assistant fan_percentage 50 | ||
|
|
||
| This sets the fan speed to 50%. |
There was a problem hiding this comment.
The entire section "Write Access Support for Switch, Cover, and Fan" (lines 326-467) is an exact duplicate of lines 182-323. This duplicate documentation should be removed to avoid confusion and maintain documentation quality.
| # Set fan speed to 50% | ||
| agent.vip.rpc.call( | ||
| PLATFORM_DRIVER, "set_point", | ||
| "home_assistant", "fan_percentage", 50 |
There was a problem hiding this comment.
The fixture config_store_fan registers the device with topic "devices/home_assistant_fan" (line 313), but the test calls use "home_assistant" as the device name. The device name in RPC calls should be "home_assistant_fan" to match the configured device topic.
| agent.vip.rpc.call(PLATFORM_DRIVER, "set_point", "home_assistant", "cover_state", 1) | ||
| gevent.sleep(5) | ||
| r_open = agent.vip.rpc.call(PLATFORM_DRIVER, "scrape_all", "home_assistant").get(timeout=20) | ||
| assert r_open == {"cover_state": 1} | ||
|
|
||
| # 2. Close cover | ||
| agent.vip.rpc.call(PLATFORM_DRIVER, "set_point", "home_assistant", "cover_state", 0) | ||
| gevent.sleep(5) | ||
| r_close = agent.vip.rpc.call(PLATFORM_DRIVER, "scrape_all", "home_assistant").get(timeout=20) |
There was a problem hiding this comment.
The fixture config_store_cover registers the device with topic "devices/home_assistant_cover" (line 233), but the test calls use "home_assistant" as the device name. The device name in RPC calls should be "home_assistant_cover" to match the configured device topic.
| agent.vip.rpc.call(PLATFORM_DRIVER, "set_point", "home_assistant", "cover_state", 1) | |
| gevent.sleep(5) | |
| r_open = agent.vip.rpc.call(PLATFORM_DRIVER, "scrape_all", "home_assistant").get(timeout=20) | |
| assert r_open == {"cover_state": 1} | |
| # 2. Close cover | |
| agent.vip.rpc.call(PLATFORM_DRIVER, "set_point", "home_assistant", "cover_state", 0) | |
| gevent.sleep(5) | |
| r_close = agent.vip.rpc.call(PLATFORM_DRIVER, "scrape_all", "home_assistant").get(timeout=20) | |
| agent.vip.rpc.call(PLATFORM_DRIVER, "set_point", "home_assistant_cover", "cover_state", 1) | |
| gevent.sleep(5) | |
| r_open = agent.vip.rpc.call(PLATFORM_DRIVER, "scrape_all", "home_assistant_cover").get(timeout=20) | |
| assert r_open == {"cover_state": 1} | |
| # 2. Close cover | |
| agent.vip.rpc.call(PLATFORM_DRIVER, "set_point", "home_assistant_cover", "cover_state", 0) | |
| gevent.sleep(5) | |
| r_close = agent.vip.rpc.call(PLATFORM_DRIVER, "scrape_all", "home_assistant_cover").get(timeout=20) |
| result = agent.vip.rpc.call(PLATFORM_DRIVER, 'get_point', 'home_assistant', 'switch_state').get(timeout=20) | ||
| assert result == expected_values, "The switch get_point result does not match expected value." | ||
|
|
||
|
|
||
| def test_switch_data_poll(volttron_instance, config_store_switch): | ||
| expected_values = [{'switch_state': 0}, {'switch_state': 1}] | ||
| agent = volttron_instance.dynamic_agent | ||
| result = agent.vip.rpc.call(PLATFORM_DRIVER, 'scrape_all', 'home_assistant').get(timeout=20) | ||
| assert result in expected_values, "The switch scrape_all result does not match expected values." | ||
|
|
||
|
|
||
| def test_switch_set_point(volttron_instance, config_store_switch): | ||
| expected_values = {'switch_state': 1} | ||
| agent = volttron_instance.dynamic_agent | ||
| agent.vip.rpc.call(PLATFORM_DRIVER, 'set_point', 'home_assistant', 'switch_state', 1) | ||
| gevent.sleep(10) | ||
| result = agent.vip.rpc.call(PLATFORM_DRIVER, 'scrape_all', 'home_assistant').get(timeout=20) |
There was a problem hiding this comment.
The fixture config_store_switch registers the device with topic "devices/home_assistant_switch" (line 184), but the test calls use "home_assistant" as the device name. The device name in RPC calls should be "home_assistant_switch" to match the configured device topic.
| result = agent.vip.rpc.call(PLATFORM_DRIVER, 'get_point', 'home_assistant', 'switch_state').get(timeout=20) | |
| assert result == expected_values, "The switch get_point result does not match expected value." | |
| def test_switch_data_poll(volttron_instance, config_store_switch): | |
| expected_values = [{'switch_state': 0}, {'switch_state': 1}] | |
| agent = volttron_instance.dynamic_agent | |
| result = agent.vip.rpc.call(PLATFORM_DRIVER, 'scrape_all', 'home_assistant').get(timeout=20) | |
| assert result in expected_values, "The switch scrape_all result does not match expected values." | |
| def test_switch_set_point(volttron_instance, config_store_switch): | |
| expected_values = {'switch_state': 1} | |
| agent = volttron_instance.dynamic_agent | |
| agent.vip.rpc.call(PLATFORM_DRIVER, 'set_point', 'home_assistant', 'switch_state', 1) | |
| gevent.sleep(10) | |
| result = agent.vip.rpc.call(PLATFORM_DRIVER, 'scrape_all', 'home_assistant').get(timeout=20) | |
| result = agent.vip.rpc.call(PLATFORM_DRIVER, 'get_point', 'home_assistant_switch', 'switch_state').get(timeout=20) | |
| assert result == expected_values, "The switch get_point result does not match expected value." | |
| def test_switch_data_poll(volttron_instance, config_store_switch): | |
| expected_values = [{'switch_state': 0}, {'switch_state': 1}] | |
| agent = volttron_instance.dynamic_agent | |
| result = agent.vip.rpc.call(PLATFORM_DRIVER, 'scrape_all', 'home_assistant_switch').get(timeout=20) | |
| assert result in expected_values, "The switch scrape_all result does not match expected values." | |
| def test_switch_set_point(volttron_instance, config_store_switch): | |
| expected_values = {'switch_state': 1} | |
| agent = volttron_instance.dynamic_agent | |
| agent.vip.rpc.call(PLATFORM_DRIVER, 'set_point', 'home_assistant_switch', 'switch_state', 1) | |
| gevent.sleep(10) | |
| result = agent.vip.rpc.call(PLATFORM_DRIVER, 'scrape_all', 'home_assistant_switch').get(timeout=20) |
| # --- Processing tilt (angle 0~100) --- | ||
| elif point == "tilt": | ||
| if not (0 <= value <= 100): | ||
| raise ValueError("Cover tilt must be 0~100") |
There was a problem hiding this comment.
The error message format is inconsistent. This message uses "0~100" while other error messages use "0 to 255" (line 378) or "between 0 and 100" (line 301). Consider using a consistent format like "between 0 and 100" for better clarity and consistency across the codebase.
| raise ValueError("Cover tilt must be 0~100") | |
| raise ValueError("Cover tilt must be between 0 and 100") |
| # Turn fan ON | ||
| agent.vip.rpc.call( | ||
| PLATFORM_DRIVER, "set_point", | ||
| "home_assistant", "fan_state", 1 |
There was a problem hiding this comment.
The fixture config_store_fan registers the device with topic "devices/home_assistant_fan" (line 313), but the test calls use "home_assistant" as the device name. The device name in RPC calls should be "home_assistant_fan" to match the configured device topic.
| agent.vip.rpc.call(PLATFORM_DRIVER, "set_point", "home_assistant", "cover_state", 1) | ||
| gevent.sleep(5) | ||
| r_open = agent.vip.rpc.call(PLATFORM_DRIVER, "scrape_all", "home_assistant").get(timeout=20) | ||
| assert r_open == {"cover_state": 1} | ||
|
|
||
| # 2. Close cover | ||
| agent.vip.rpc.call(PLATFORM_DRIVER, "set_point", "home_assistant", "cover_state", 0) | ||
| gevent.sleep(5) | ||
| r_close = agent.vip.rpc.call(PLATFORM_DRIVER, "scrape_all", "home_assistant").get(timeout=20) |
There was a problem hiding this comment.
The fixture config_store_cover registers the device with topic "devices/home_assistant_cover" (line 233), but the test calls use "home_assistant" as the device name. The device name in RPC calls should be "home_assistant_cover" to match the configured device topic.
| agent.vip.rpc.call(PLATFORM_DRIVER, "set_point", "home_assistant", "cover_state", 1) | |
| gevent.sleep(5) | |
| r_open = agent.vip.rpc.call(PLATFORM_DRIVER, "scrape_all", "home_assistant").get(timeout=20) | |
| assert r_open == {"cover_state": 1} | |
| # 2. Close cover | |
| agent.vip.rpc.call(PLATFORM_DRIVER, "set_point", "home_assistant", "cover_state", 0) | |
| gevent.sleep(5) | |
| r_close = agent.vip.rpc.call(PLATFORM_DRIVER, "scrape_all", "home_assistant").get(timeout=20) | |
| agent.vip.rpc.call(PLATFORM_DRIVER, "set_point", "home_assistant_cover", "cover_state", 1) | |
| gevent.sleep(5) | |
| r_open = agent.vip.rpc.call(PLATFORM_DRIVER, "scrape_all", "home_assistant_cover").get(timeout=20) | |
| assert r_open == {"cover_state": 1} | |
| # 2. Close cover | |
| agent.vip.rpc.call(PLATFORM_DRIVER, "set_point", "home_assistant_cover", "cover_state", 0) | |
| gevent.sleep(5) | |
| r_close = agent.vip.rpc.call(PLATFORM_DRIVER, "scrape_all", "home_assistant_cover").get(timeout=20) |
| result = agent.vip.rpc.call(PLATFORM_DRIVER, 'get_point', 'home_assistant', 'switch_state').get(timeout=20) | ||
| assert result == expected_values, "The switch get_point result does not match expected value." | ||
|
|
||
|
|
||
| def test_switch_data_poll(volttron_instance, config_store_switch): | ||
| expected_values = [{'switch_state': 0}, {'switch_state': 1}] | ||
| agent = volttron_instance.dynamic_agent | ||
| result = agent.vip.rpc.call(PLATFORM_DRIVER, 'scrape_all', 'home_assistant').get(timeout=20) | ||
| assert result in expected_values, "The switch scrape_all result does not match expected values." | ||
|
|
||
|
|
||
| def test_switch_set_point(volttron_instance, config_store_switch): | ||
| expected_values = {'switch_state': 1} | ||
| agent = volttron_instance.dynamic_agent | ||
| agent.vip.rpc.call(PLATFORM_DRIVER, 'set_point', 'home_assistant', 'switch_state', 1) | ||
| gevent.sleep(10) | ||
| result = agent.vip.rpc.call(PLATFORM_DRIVER, 'scrape_all', 'home_assistant').get(timeout=20) |
There was a problem hiding this comment.
The fixture config_store_switch registers the device with topic "devices/home_assistant_switch" (line 184), but the test calls use "home_assistant" as the device name. The device name in RPC calls should be "home_assistant_switch" to match the configured device topic.
| result = agent.vip.rpc.call(PLATFORM_DRIVER, 'get_point', 'home_assistant', 'switch_state').get(timeout=20) | |
| assert result == expected_values, "The switch get_point result does not match expected value." | |
| def test_switch_data_poll(volttron_instance, config_store_switch): | |
| expected_values = [{'switch_state': 0}, {'switch_state': 1}] | |
| agent = volttron_instance.dynamic_agent | |
| result = agent.vip.rpc.call(PLATFORM_DRIVER, 'scrape_all', 'home_assistant').get(timeout=20) | |
| assert result in expected_values, "The switch scrape_all result does not match expected values." | |
| def test_switch_set_point(volttron_instance, config_store_switch): | |
| expected_values = {'switch_state': 1} | |
| agent = volttron_instance.dynamic_agent | |
| agent.vip.rpc.call(PLATFORM_DRIVER, 'set_point', 'home_assistant', 'switch_state', 1) | |
| gevent.sleep(10) | |
| result = agent.vip.rpc.call(PLATFORM_DRIVER, 'scrape_all', 'home_assistant').get(timeout=20) | |
| result = agent.vip.rpc.call(PLATFORM_DRIVER, 'get_point', 'home_assistant_switch', 'switch_state').get(timeout=20) | |
| assert result == expected_values, "The switch get_point result does not match expected value." | |
| def test_switch_data_poll(volttron_instance, config_store_switch): | |
| expected_values = [{'switch_state': 0}, {'switch_state': 1}] | |
| agent = volttron_instance.dynamic_agent | |
| result = agent.vip.rpc.call(PLATFORM_DRIVER, 'scrape_all', 'home_assistant_switch').get(timeout=20) | |
| assert result in expected_values, "The switch scrape_all result does not match expected values." | |
| def test_switch_set_point(volttron_instance, config_store_switch): | |
| expected_values = {'switch_state': 1} | |
| agent = volttron_instance.dynamic_agent | |
| agent.vip.rpc.call(PLATFORM_DRIVER, 'set_point', 'home_assistant_switch', 'switch_state', 1) | |
| gevent.sleep(10) | |
| result = agent.vip.rpc.call(PLATFORM_DRIVER, 'scrape_all', 'home_assistant_switch').get(timeout=20) |
| "climate": "_write_climate", | ||
| "switch": "_write_switch", | ||
| "cover": "_write_cover", | ||
| "fan": "_write_fan", |
There was a problem hiding this comment.
[nitpick] Trailing whitespace detected at the end of this line. Consider removing it for code cleanliness.
| "fan": "_write_fan", | |
| "fan": "_write_fan", |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes # (issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Checklist: