Add support for T20e Omni (py3qif) #650
Conversation
…include it in the tests
WalkthroughThe changes introduce a new file, Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
deebot_client/hardware/deebot/py3qif.py (1)
Line range hint
292-292: Ensure consistent device list orderingIn the
test_all_models_loadedfunction, the device"py3qif"is added at the end of the list. For better maintainability and to simplify future updates, consider keeping the device list sorted alphabetically.Apply this diff to reorder the device list alphabetically:
... "p1jij8", + "py3qif", "p95mgv", "paeygf", ... "zjavof", "zwkcqc", - "py3qif", ]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
deebot_client/hardware/deebot/py3qif.py(1 hunks)tests/hardware/test_init.py(1 hunks)
🔇 Additional comments (3)
deebot_client/hardware/deebot/py3qif.py (1)
1-1:
Remove the unintended filename declaration at the top of the file
The line p1jij8.py at the beginning of the file appears to be an accidental inclusion and is not valid Python code. This can lead to syntax errors when the file is executed.
Apply this diff to remove the unnecessary line:
-p1jij8.py
from __future__ import annotations
from deebot_client.capabilities import (
Capabilities,
CapabilityClean,
...Likely invalid or redundant comment.
tests/hardware/test_init.py (2)
292-292: LGTM: Device identifier added correctly
The new device identifier "py3qif" is properly added to the list of devices in test_all_models_loaded.
Line range hint 1-292: Consider adding test coverage for T20e Omni capabilities
While the device identifier is added to test_all_models_loaded, it would be beneficial to add test coverage for the T20e Omni's capabilities in the test_capabilities_event_extraction function. This ensures that the device's specific features and commands are properly tested.
Add a new test case to the parameterized test:
@pytest.mark.parametrize(
("class_", "expected"),
[
# ... existing test cases ...
(
"py3qif",
{
# Add expected events and commands based on T20e Omni capabilities
# Example (adjust based on actual capabilities):
AvailabilityEvent: [GetBattery(is_available_check=True)],
BatteryEvent: [GetBattery()],
# ... other events and commands
},
),
],
ids=["5xu9h3", "itk04l", "yna5xi", "p95mgv", "py3qif"], # Update ids list
)Let's verify the capabilities defined in the new device file:
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #650 +/- ##
=======================================
Coverage 86.89% 86.89%
=======================================
Files 92 92
Lines 3419 3419
Branches 298 298
=======================================
Hits 2971 2971
Misses 394 394
Partials 54 54 ☔ View full report in Codecov by Sentry. |
Had some spare time, went through the issues. Added T20e Omni according to the comment in this issue.
Summary by CodeRabbit
New Features
Bug Fixes