SmartThingsEdgeDrivers icon indicating copy to clipboard operation
SmartThingsEdgeDrivers copied to clipboard

fix-an-issue-of-thermostat-does-not-support-fan-speed

Open pInksenberg opened this issue 1 year ago • 6 comments

Check all that apply

Type of Change

  • [ ] WWST Certification Request
    • If this is your first time contributing code:
      • [ ] I have reviewed the README.md file
      • [ ] I have reviewed the CODE_OF_CONDUCT.md file
      • [ ] I have signed the CLA
    • [ ] I plan on entering a WWST Certification Request or have entered a request through the WWST Certification console at developer.smartthings.com
  • [x] Bug fix
  • [ ] New feature
  • [ ] Refactor

Checklist

  • [x] I have performed a self-review of my code
  • [ ] I have commented my code in hard-to-understand areas
  • [x] I have verified my changes by testing with a device or have communicated a plan for testing
  • [ ] I am adding new behavior, such as adding a sub-driver, and have added and run new unit tests to cover the new behavior

Description of Change

Problem: The LengCeoi HAVC Remote were recently tested in ST App, which include Thermostat and Fan device type. However the capabilities in plugin shows incorrectly

Expectation Result: The capabilities of thermostat, humidity, fan speed, thermostat mode, thermostat fan mode and set temperature show in the ST plugin.

Current Issue: The plugin show fan mode and fan speed.

Analysis: Is it found that the edge driver(matter-thermostat) will re-assign a new profile while driver initialize, and go to the Fan logic, the fan logic does not check if the device support thermostat, or humidity, which means it will re-assign the “fan-generic” for driver.

The profile name re-assign logic is changed to check if the fan speed is supported.

Related tickets: https://smartthings.atlassian.net/browse/MTR-866

Summary of Completed Tests

pInksenberg avatar Dec 10 '24 09:12 pInksenberg

Duplicate profile check: Passed - no duplicate profiles detected.

github-actions[bot] avatar Dec 10 '24 09:12 github-actions[bot]

Invitation URL: https://bestow-regional.api.smartthings.com/invite/Boj0m8J7amMA

github-actions[bot] avatar Dec 10 '24 09:12 github-actions[bot]

Test Results

   64 files    403 suites   0s ⏱️ 2 006 tests 2 006 ✅ 0 💤 0 ❌ 3 468 runs  3 468 ✅ 0 💤 0 ❌

Results for commit e00e92b6.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Dec 10 '24 09:12 github-actions[bot]

File Coverage
All files 82% :x:
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-thermostat/src/init.lua 84% :x:
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-thermostat/src/embedded-cluster-utils.lua 42% :x:

Minimum allowed coverage is 90%

Generated by :monkey: cobertura-action against e00e92b6c527962316923234af1d8ebf4e3a1588

github-actions[bot] avatar Dec 10 '24 09:12 github-actions[bot]

We don't currently support the fan speed capability for thermostat, and that is on purpose. We have internally discussed adding this, but for now we do not plan to add this feature for thermostats.

hcarter-775 avatar Jan 07 '25 17:01 hcarter-775

I believe similar functionality could be achieved by simply reordering the if-else logic in do_configure. Specifically, if we put the elseif device_type == FAN_DEVICE_TYPE_ID then at the bottom of the ordering, then the logic would see this check first elseif #thermostat_eps > 0 then and would profile based on that logic, only going to the FAN_DEVICE_TYPE_ID logic in the case that #thermostat_eps == 0.

hcarter-775 avatar Jan 07 '25 17:01 hcarter-775