-
-
Notifications
You must be signed in to change notification settings - Fork 89
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
Add Support for Deebot N8+ #628
Conversation
WalkthroughThe changes involve the introduction of a new file, Changes
Possibly related PRs
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
deebot_client/hardware/deebot/7bryc5.py (1)
106-110
: Consider refactoring repeatedCapabilitySetEnable
definitions to reduce code duplicationMultiple
CapabilitySetEnable
instances with similar structure are defined for capabilities such ascontinuous
,preference
,multi_state
,advanced_mode
,carpet_auto_fan_boost
, andtrue_detect
. Refactoring to use a helper function or a factory method could improve maintainability by reducing code duplication and enhancing readability.Also applies to: 113-115, 156-158, 167-169, 170-174, 175-177
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
deebot_client/hardware/deebot/7bryc5.py
(1 hunks)
🔇 Additional comments (1)
deebot_client/hardware/deebot/7bryc5.py (1)
95-198
: LGTM!
The capabilities for the DEEBOT N8+ are correctly defined, and the code aligns with the existing codebase structure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
tests/hardware/test_init.py
(1 hunks)
🔇 Additional comments (2)
tests/hardware/test_init.py (2)
261-261
: LGTM on adding the N8+ device identifier.
The device identifier "7bryc5" has been correctly added to the list of supported devices.
261-261
: Verify N8+ capabilities implementation.
Let's verify that the N8+ capabilities are properly implemented in the codebase.
✅ Verification successful
Let me gather more information about the N8+ capabilities implementation.
N8+ capabilities are properly implemented
The implementation in deebot_client/hardware/deebot/7bryc5.py
shows a complete and comprehensive implementation of N8+ capabilities including:
- All core vacuum functionalities (clean, charge, fan speed)
- Advanced features (true detect, carpet auto fan boost)
- Map management capabilities
- Life span monitoring for components
- Water system controls
- Statistics and state monitoring
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify N8+ capabilities implementation
# Check if the device file exists
echo "Checking for N8+ device file..."
fd "7bryc5.py$"
# Check capabilities implementation
echo "Checking N8+ capabilities implementation..."
rg -A 10 "7bryc5" . --type py
# Look for any TODO or FIXME comments in the N8+ implementation
echo "Checking for TODO/FIXME comments..."
rg "TODO|FIXME" . --type py
Length of output: 1417
Script:
#!/bin/bash
# Check the implementation details in the 7bryc5.py file
echo "Checking N8+ implementation details..."
cat deebot_client/hardware/deebot/7bryc5.py
# Check for any references to N8+ capabilities
echo "Checking for N8+ capability references..."
rg -i "n8\+" . --type py
# Check for capability-related code patterns
echo "Checking capability patterns..."
ast-grep --pattern 'class $_ {
$$$
capabilities($$$) {
$$$
}
$$$
}'
Length of output: 7522
@@ -258,6 +258,7 @@ def test_all_models_loaded() -> None: | |||
"5xu9h3", | |||
"626v6g", | |||
"77atlz", | |||
"7bryc5", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add test coverage for N8+ specific capabilities.
While the device identifier has been added, there's no test coverage for the N8+ specific capabilities in the test_capabilities_event_extraction
function. Please add a test case for the "7bryc5" device to verify its capabilities and commands.
Example addition to the test_capabilities_event_extraction parametrize:
(
"7bryc5",
{
# Add expected events and commands specific to N8+
# Example (adjust based on actual capabilities):
AvailabilityEvent: [GetBattery(is_available_check=True)],
BatteryEvent: [GetBattery()],
# ... other capabilities
},
)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #628 +/- ##
==========================================
+ Coverage 86.42% 86.55% +0.13%
==========================================
Files 88 89 +1
Lines 3301 3333 +32
Branches 298 298
==========================================
+ Hits 2853 2885 +32
Misses 394 394
Partials 54 54 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @brendanm720 👍
Hi everyone, I really appreciate your efforts to get our Deebots working with HA again. However, could you provide a unique procedure (the simplest possible) to understand, even for dumb users like me, how to fix these files? |
Just updated to the latest released which included this yet still running into:
N8+ should be working with this? |
it looks like the class id for the "black" version is different from the "white" version even though electronically they're the same thing. I will submit another PR for the black version for you. |
Amazing Thank you so much!
|
Good Evening!
Trying this again!
Please merge this file to add support for the Deebot N8+. I have tested this with my Deebot N8+ install and it seems to be working as expected.
Thank you.
Summary by CodeRabbit