Improve test coverage and backwards compatibility for matter switch tests #2028
Conversation
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
|
Channel deleted. |
Test Results 66 files 424 suites 0s ⏱️ Results for commit 32a9f65. ♻️ This comment has been updated with latest results. |
|
Minimum allowed coverage is Generated by 🐒 cobertura-action against 32a9f65 |
nickolas-deboom
force-pushed
the
matter-switch-fix-test-cases-for-lower-lua-libs
branch
2 times, most recently
from
63574b2 to
7188758
Compare
nickolas-deboom
force-pushed
the
matter-switch-fix-test-cases-for-lower-lua-libs
branch
3 times, most recently
from
4ea4089 to
a07ec42
Compare
Add backwards compatibility for matter switch test cases down to lua libs version v10. Most of the issues were from test cases expecting native handler registrations, which was added in v11. Also, running the test cases against lower lua libs caught a bug in the driver code for lua libs version v10 and lower, which prevented the energyMeter capability from being emitted. This is also fixed by this PR. It might be possible to extend support to even lower versions, but there are a few additional api changes that would need to be accounted for. Also note that to run these tests against lower versions, any capabilities that are not available in that version of the lua libs would need to be manually added to prevent test failures.
nickolas-deboom
force-pushed
the
matter-switch-fix-test-cases-for-lower-lua-libs
branch
from
a07ec42 to
be5f48e
Compare
This commit removes some unreachable code and also adds new test cases to improve code coverage.
nickolas-deboom
force-pushed
the
matter-switch-fix-test-cases-for-lower-lua-libs
branch
2 times, most recently
from
daf4874 to
f8ae614
Compare
nickolas-deboom
changed the title
nickolas-deboom
changed the title
nickolas-deboom
force-pushed
the
matter-switch-fix-test-cases-for-lower-lua-libs
branch
2 times, most recently
from
409c457 to
b9c573d
Compare
Add new test cases to cover areas missing test coverage.
nickolas-deboom
force-pushed
the
matter-switch-fix-test-cases-for-lower-lua-libs
branch
from
b9c573d to
32a9f65
Compare
nickolas-deboom
commented
Apr 18, 2025
| end | ||
|
|
||
| local function mired_to_kelvin(value, minOrMax) | ||
| if value == 0 then -- shouldn't happen, but has |
Contributor
Author
There was a problem hiding this comment.
This code is unreachable because 0 is outside of the range of "sane" values checked at the call site
nickolas-deboom
commented
Apr 18, 2025
| elseif minOrMax == COLOR_TEMP_MAX then | ||
| else -- minOrMax = COLOR_TEMP_MAX | ||
| return utils.round(MIRED_KELVIN_CONVERSION_CONSTANT / (kelvin_step_size * (value - 1)) - rounding_value) * kelvin_step_size | ||
| else |
Contributor
Author
There was a problem hiding this comment.
This else is unreachable because this function is only called with COLOR_TEMP_MIN or COLOR_TEMP_MAX, so it can be removed
nickolas-deboom
commented
Apr 18, 2025
| if ep.endpoint_id == endpoint_id then | ||
| for _, dt in ipairs(ep.device_types) do | ||
| if dt.device_type_id == DIMMABLE_LIGHT_DEVICE_TYPE_ID then | ||
| for _, fingerprint in ipairs(child_device_profile_overrides_per_vendor_id[0x115F]) do |
Contributor
Author
There was a problem hiding this comment.
This Aqara device uses device type 0x100, while DIMMABLE_LIGHT_DEVICE_TYPE_ID is 0x101, so this code can be removed
nickolas-deboom
commented
Apr 18, 2025
| end | ||
|
|
||
| local function configure_buttons(device) | ||
| if device.network_type == device_lib.NETWORK_TYPE_CHILD then |
Contributor
Author
There was a problem hiding this comment.
This check is already done at both call sites of configure_buttons so it can be removed
Contributor
|
do you think this is worth a rebase? Should we close this? Just checking since it's been open for ~6 months. |
Contributor
Author
I would say probably not because backwards compatibility was broken by scripting engine changes. Closing |
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.
Type of Change
Description of Change
Add backwards compatibility for matter switch test cases down to lua libs version v10 and improve overall test coverage. Most of the issues were from test cases expecting native handler registrations, which was added in v11. This PR also fixes a bug in the driver code for lua libs version v10 and lower, which prevented the energyMeter capability from being emitted.
It might be possible to extend support to even lower versions, but there are a few additional api changes that would need to be accounted for.
Also note that to run these tests against lower versions, any capabilities that are not available in that version of the lua libs would need to be manually added to prevent test failures.