Add abode support for CUE automations#31514
Conversation
MartinHjelmare
changed the title
Codecov Report
@@ Coverage Diff @@
## dev #31514 +/- ##
==========================================
- Coverage 94.75% 94.72% -0.03%
==========================================
Files 772 778 +6
Lines 55885 56109 +224
==========================================
+ Hits 52953 53152 +199
- Misses 2932 2957 +25
Continue to review full report at Codecov.
|
|
First attempt at adding tests for the Abode component. I'm still figuring out how to write tests but for now, I'm attempting to test that the Abode component is initialized correctly and that devices are setup appropriately. I don't have all of the different Abode devices in my home setup but I'll continue to expand the abode_devices.json fixtures as I obtain more device JSON data. I also haven't implemented any checking of abode services or state changes yet. |
|
More test updates. Here are the known "issues" with tests:
Anyways, hopefully this is a decent start to tests for the abode integration. I'll continue to make additions/modifications as required for this PR to be merged, but I also continue to plan on working on tests as I learn more to get 100% coverage for abode. |
| async def test_connection_error(hass, requests_mock): | ||
| """Test exception when unable to connect to Abode.""" | ||
| requests_mock.post(CONST.LOGIN_URL, exc=ConnectTimeout) | ||
| # Tried using 'with pytest.raises(ConnectTimeout):' but |
There was a problem hiding this comment.
For whatever reason, ConnectTimeout is not being raised. The strange part is I know the request_mock.post(CONST.LOGIN_URL, exc=ConnectTImeout) line is partially working because when I run the test, I can see it's executing the appropriate part of the abode module and logging the correct message. However, where it prints out the exception in that log message, it's blank. I've tried a number of different things such as mocking abodepy.Abode.login and setting a side effect exception but it still doesn't work.
| self._automation.set_active(True) | ||
| """Enable the automation.""" | ||
| if self._automation.enable(True): | ||
| self.schedule_update_ha_state() |
There was a problem hiding this comment.
This is required based on there being no web socket events for automations being disabled or enabled (unlike normal devices in Abode). How the abode integration is currently setup, when a device is switched on/off, it calls a method in abodepy that will execute the command and also immediately refresh the device's JSON data. However, the abode integration waits until it receives a web socket event of the device state changing to execute a call back which calls self.schedule_update_ha_state(). Again, this doesn't happen with automations hence the reason I'm calling self.schedule_update_ha_state() here.
As a side note, it seems like doing this on all devices in the abode integration would make sense such that Home Assistant has the correct state immediately (the JSON response is much faster than waiting for the web socket event). In some situations where the device web socket event is slow to be received, you will see a switch in HA go from on, to off, to back on when you simply turned it on. The downside is we're basically updating the device state in HA twice, once immediately after the device state was changed and again when we receive the web socket event. I'm not sure if that's considered bad practice, but it seems better from a UX perspective.
| async def test_set_color(hass, requests_mock): | ||
| """Test the color can be set.""" | ||
| await setup_platform(hass, LIGHT_DOMAIN) | ||
| # light.turn_on service is calling `turn_on` in switch.py and light.py |
There was a problem hiding this comment.
Edit: Made some slight modifications to test_light.py but the comment below is still valid (not outdated).
The fix for this is simply adding return statements in AbodeLight.turn_on in light.py. The issue is the else: statement is being matched every time which is not desired. If turn_on is called with any color or brightness information, it should execute the appropriate if statement and return. Otherwise, we're making two separate calls to Abode's servers which is not the intent.
I have the fixes and changes to test_light.py on my local repo but I haven't committed them based on the dev docs stating:
Do not mix clean-ups and new features in a single pull request.
I can either way until this PR is merged or since it's a very small change, add them to this one if desired.
2 participants
Breaking change
Abode is retiring its legacy automations and quick actions on 28 Feb (see Abode support article here). This change removes support for legacy automations and quick actions while adding support for CUE automations which will show up in Home Assistant as switches (same as previous implementation). Abode's replacement for quick actions is manually triggered CUE automations which can be called through Home Assistant service
abode.trigger_automation(previously labeledabode.trigger_quick_action).Proposed change
Removed support for Abode's legacy automations and quick actions being retired on 28 Feb. Adds support for Abode's new CUE automations which will show up as switches in Home Assistant (same as previously implementation). CUE automations can be called using the
abode.trigger_automationservice. Additionally, added tests for the Abode integration, removed some unused variables.Type of change
Additional information
Checklist
black --fast homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all..coveragerc.The integration reached or maintains the following Integration Quality Scale: