Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Eve Energy: Improve the performance when the device is off #1087

Merged

Conversation

Timac
Copy link
Contributor

@Timac Timac commented Nov 24, 2023

  • Don't the power reports of the device if the device is off
  • In the timer, we now check if the device is off using device:get_latest_state()
  • Set the power to 0 before the read is skipped so that the power is correctly displayed and not using a stale value

- Don't the power reports of the device if the device is off
- In the timer, we now check if the device is off using device:get_latest_state()
- Set the power to 0 before the read is skipped so that the power is correctly displayed and not using a stale value
Copy link

github-actions bot commented Nov 24, 2023

Test Results

     55 files  +  1     347 suites  +1   0s ⏱️ ±0s
1 636 tests +12  1 636 ✔️ +12  0 💤 ±0  0 ±0 
2 864 runs  +13  2 864 ✔️ +13  0 💤 ±0  0 ±0 

Results for commit 3cc0a14. ± Comparison against base commit 7815132.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Nov 24, 2023

File Coverage
All files 93%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/init.lua 90%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/eve-energy/init.lua 97%

Minimum allowed coverage is 90%

Generated by 🐒 cobertura-action against 3cc0a14

Copy link

github-actions bot commented Nov 24, 2023

Channel deleted.

- Add unit test to test the on attribute
@ctowns ctowns self-requested a review December 7, 2023 16:36
Copy link
Contributor

@ctowns ctowns left a comment

Choose a reason for hiding this comment

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

In order to satisfy the ST energy requirement, we still need to report the power consumption every 15 minutes (even when the device is off). I think this could be accomplished by moving the powerConsumptionReport to it's own timer that runs every 15 minutes. This way it is not tied to the 1-minute timer (that could be stopped when the device is off with these changes) and the powerConsumptionReport will continue to report regardless of whether the device is on or off.

This would essentially mean moving the second half of the updateEnergyMeter function (the part that only runs every 15 minutes) to his own function that is called every 15 minutes. This way it is decoupled from the 1-minute poll. What are your thoughts on this?

@@ -109,6 +109,12 @@ local function create_poll_schedule(device)
-- The powerConsumption report needs to be updated at least every 15 minutes in order to be included in SmartThings Energy
-- Eve Energy generally report changes every 10 or 17 minutes
local timer = device.thread:call_on_schedule(TIMER_REPEAT, function()
-- We want to prevent to read the power reports of the device if the device is off
local is_off = device:get_latest_state("main", capabilities.switch.ID, capabilities.switch.switch.NAME) == "off"
if is_off then
Copy link
Contributor

Choose a reason for hiding this comment

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

With this change, we will only ever be turning off the polling is the device is off during device_init, which is the only place create_poll_schedule is called. However, we want to be able to remove the polling anytime the device is off. I would suggest moving this logic to the on/off handlers and then removing the poll timer when the device is off (see device_removed for example) and then starting the timer again if the device is switched on and a timer is not already running (i.e. poll_timer == nil)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At the moment, the timer will indeed always run but the requestData() function won’t be called when the device is off.

We can change the behavior as suggested:

  • stop the timer when the device is off and restart it when the device is switched on
  • use a distinct timer to report the power consumption every 15 minutes

Timac and others added 4 commits December 8, 2023 15:07
- Create the 1-minute timer when the device is switched on (if the timer doesn't exist yet)
- Delete the 1-minute timer when the device is turned off
- The totle consumption is saved in the variable LATEST_TOTAL_CONSUMPTION_WH
- There is now a 15-minutes timer to send the powerConsumptionReport report to satisfy the ST energy requirement
@Timac Timac requested a review from ctowns December 21, 2023 11:21
@ctowns ctowns merged commit 81d9dfe into SmartThingsCommunity:main Jan 9, 2024
11 checks passed
@Timac Timac deleted the feature/eve-performance-device-off branch January 10, 2024 12:36
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.

2 participants