Add X1 Omni support (1vxt52) #611
Conversation
WalkthroughThe pull request introduces a new file, Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (2)tests/hardware/test_init.py (2)
The addition of "1vxt52" to the supported devices list aligns with the X1 Omni model mentioned in the PR objectives. Line range hint The test file lacks specific test cases for the X1 Omni (1vxt52) model's capabilities. Since you mentioned the device works with both X2 Omni and X1 Turbo configurations, we should:
Let's verify which configuration is being used for the new model: Based on your testing results showing X1 Turbo being more aligned with X1 Omni's feature set, here's a suggested test addition: @pytest.mark.parametrize(
("class_", "expected"),
[
("not_specified", lambda: None),
("yna5xi", lambda: DEVICES["yna5xi"]),
+ ("1vxt52", lambda: DEVICES["1vxt52"]),
],
)
async def test_get_static_device_info(...)
@pytest.mark.parametrize(
("class_", "expected"),
[
# existing test cases...
+ (
+ "1vxt52",
+ {
+ # Copy capabilities from X1 Turbo configuration
+ # Adjust based on actual supported features
+ }
+ ),
],
ids=["5xu9h3", "itk04l", "yna5xi", "p95mgv", "1vxt52"],
)
async def test_capabilities_event_extraction(...)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 (
|
|
FOLLOW THESE AT YOUR OWN RISK TO TEST If anyone spots this and is willing to risk completely breaking their HA install here are the steps for testing this locally on HAOS 2024.12.0:
FOLLOW THESE AT YOUR OWN RISK TO TEST |
![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/1vxt52.py (1)
Line range hint
3-97: Consider reusing existing device configurations to reduce duplicationSince the capabilities of the X1 Omni are similar to those of the X1 Turbo (
2o4lnm), you could import the existing configuration instead of duplicating the code. This approach reduces code duplication and simplifies maintenance.Here’s how you might adjust the code:
from .2o4lnm import DEVICES as PARENT_DEVICES, short_name as parent_short_name DEVICES[short_name(__name__)] = PARENT_DEVICES[parent_short_name(__name__)]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
deebot_client/hardware/deebot/1vxt52.py(1 hunks)
🔇 Additional comments (1)
deebot_client/hardware/deebot/1vxt52.py (1)
1-1:
Remove unintended filename reference
The line 2o4lnm.py appears at the beginning of the file, which seems unintended and will cause a syntax error. Please remove this line.
Apply this diff to fix the issue:
-2o4lnm.pyLikely invalid or redundant comment.
There is no "correct" way to determine the features properly as Ecovacs is not providing any public API documentation. Therefore, I used the "try and error" approach in the past. I'm also logging the traffic between the app and the Ecovacs server to see if I can spot new commands but for that you need a man in the middle proxy setup, which is also not trivial |
edenhaus
changed the title
|
Please fix the tests by applying: diff --git a/tests/hardware/test_init.py b/tests/hardware/test_init.py
index b4039cb..24bf9df 100644
--- a/tests/hardware/test_init.py
+++ b/tests/hardware/test_init.py
@@ -250,6 +250,7 @@ def test_all_models_loaded() -> None:
"""Test that all models are loaded."""
_load()
assert list(DEVICES) == [
+ "1vxt52",
"2ap5uq",
"2o4lnm",
"4vhygi",or give me push access to your repo |
I tried this to make it work my Deebot T9, but I got an error: How can I check correct file to my deebot T9? is there a way to make it work? Many Thanks |
|
@gdgib |
|
I am a Home Assistant user and I was trying the Ecovacs integration for the first time today, only to find that my model (X1 Omni) was not supported. That led me to this repository, where I checked open PRs and saw this one from the same day proposing to add support for exactly my model 🎉 So thanks @gdgib! So far, trying out this PR in my HA seems to work: it connected my X1 Omni to HA, where previously it didn't work (i.e. no entities/devices were previously discovered by Ecovacs integration). Some entities don't seem to be enabled for this model, but I didn't expect all entities to be available. The most important information from my vaccum, such as its battery level and the size of area cleaned, as well as the current cleaning status, seems to be read correctly. Should I test something specific on my end?
Thanks for this, but it took me a while to interpret these instructions. I tried ssh-ing into HA the way I always do, using the "Terminal & SSH" addon, but Then, once in SSH session on the HA host system, typing However, I realized that within this shell I can just access docker exec "$(docker ps -f name=homeassistant -q)" ln -svfT 2o4lnm.py /usr/local/lib/python3.13/site-packages/deebot_client/hardware/deebot/1vxt52.pyThen I restarted my HA and the changes in this PR took effect for me 👍 |
|
@edenhaus Thanks for the answer. |
Until tomorrow 12 CET. @gdgib Please checkout #611 (comment) to get this PR merged as soon as possible. |
Thank you so much and following the one-liner my X1 Omni is now back online. I tried to also use the same method to put my Ecovacs U2 Pro back online! @edenhaus Hi, I am not sure how to create a PR but I can confirm the same procedure works for Ecovacs U2 PRO (7j1tu6). What I did was to use the same one-liner and had 1vxt52.py replaced with 7j1tu6.py, reboot HA and I am completely back in the game. May I ask if this can be included from the next release? Thank you! |
|
@edenhaus added you to the fork AND updated the tests for good measure. Thanks for the fast turn around. @willliamchan if you look through the files in https://github.com/DeebotUniverse/client.py/tree/dev/deebot_client/hardware/deebot, you'll find a bunch are just one-line symlinks. Ignore those for now. Do any of the longer files go with a deebot model similar to the U2 Pro? I ask because while the hack you're using will work for the basics, I don't think it's quite right since I think the X1 series and U2 series seem to have different features. I strongly suggest opening an issue (https://github.com/DeebotUniverse/client.py/issues) for the U2 Pro if there isn't one already, since it's a separate device. I might be able to help with this when I'm not on my lunch break. |
Hi, yes i think you're right that U2 Pro is more like a basic one without rich features like X1 Omni. I am not sure which file look alike U2 Pro to be honest, however I tried to enable all the "disabled entities" and take notes of the below that shouldn't belong to U2 Pro and they are:- Configuration Diagnostic |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #611 +/- ##
=======================================
Coverage 86.42% 86.42%
=======================================
Files 88 88
Lines 3301 3301
Branches 298 298
=======================================
Hits 2853 2853
Misses 394 394
Partials 54 54 ☔ View full report in Codecov by Sentry. |
|
I'll make you a deal @willliamchan, if you open a separate issue so that we can keep each model to its own PR/issue, then I'll try to hack something together for you today. All you have to do is go to is create an issue, and summarize the last two comments in it. Heck you can even just paste links in there! Honestly, if you don't do that I'll probably do that for you too, but it's a good habit to learn when you need something from an open source project to file a separate issue for each request. It really helps our sanity. |
|
Thank you @edenhaus ! Can't tell you how much I appreciate all your hard work on this code base, and the integration. |
Thank you very much Greg, I think I've managed to create a new issue and here is the link - #629 |
I've got an X1 Omni (1vxt52), and obviously with the recent changes it no longer shows up in HA. My bot seems to work well enough with both the X2 Omni and X1 Turbo files, but I think the X1 Turbo is closer in feature set, both according to marketing and testing.
To test, I just hacked the symlink into my running HA instance and restarted HA. 2o4lnm seems a little better fit, but e6ofmn handled at least the basics, FWIW.
I admit I would LOVE a guide on how to properly determine the features in this file, because in the absence of that I'm "hacking" more than "coding" and that makes me justifiably nervous. Just because everything works doesn't mean it works for the right reasons, you know?
Let me know if you've got other steps I can take to make sure this is really working, please!
Summary by CodeRabbit