Force parameters to be an empty list if none is given #408
Conversation
This file contains 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
This seems to work fine with yeelight and gen1 vacuum, however, more testing is needed with other devices and/or this change should only be done for the vacuum. Related to #348 / Xiaowa E352
This was referenced Nov 10, 2018
|
Most devices doesn't use parameterless commands: Devices with parameterless commands: The |
|
You are the MAN 🥇 mirobo home 🛑 mirobo install-sound ❓ I did not try this mirobo sound 🛑 mirobo spot 👌 mirobo timer 👌 |
|
Glad to hear it's working, thanks for extensive reporting! Sound and serialnumber seems to have changed their payloads, so that needs a bit adaptation, but is not a big issue. I'm more worried about why some of the commands (esp. stop) do not respond, maybe you executed too many commands in a row and the vacuum refused some of those? @syssi I'm not sure if this PR should be merged, maybe it'd be safer simply to adapt vacuum to pass an empty list to avoid breaking anything. I just created this PR to make it easier to test if this is the issue on some vacuums :-) |
|
I dont think so, I tried within longer time range, I can start, pause, spot
cleanup, however stop and home is not working. Can I see even more output
somewhere?
…On Sun, Nov 11, 2018, 15:08 Teemu R. ***@***.*** wrote:
Glad to hear it's working, thanks for extensive reporting! Sound and
serialnumber seems to have changed their payloads, so that needs a bit
adaptation, but is not a big issue. I'm more worried about why some of the
commands (esp. stop) do not respond, maybe you executed too many commands
in a row and the vacuum refused some of those?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#408 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AGFfnIaueNnYMYfd4wyyIaaWw29t8Fckks5uuC9igaJpZM4YYPdY>
.
|
|
Not really, as the device is not responding anything to those requests. Maybe the commands are different for those on your device? Or maybe they expect empty parameter list (then they should work without this PR, though)? |
|
I tried different server with mirobo version 0.4.3 |
|
Update: |
|
Ok, maybe the stop is then removed (I suppose pause does mostly the same though). I'm not sure why |
|
I changed in vacuum.py , cannot push from git and also now it is working as supposed. |
|
roborock.vacuum.c1 here This seems to work with c1 without any --nextgen. Apart from: mirobo --debug serial-number mirobo --debug sound And if I ask for status after these commands throwing, status also throws: mirobo --debug status Asking for successful stuff (like stop) will return a correct status once again, I guess maybe status returns last error which we're unable to parse or something like that. |
|
stop is not stopping cleaning for me, only pause, but the command itself works fine, returns ok. Mi home app seems to do the same, firstly it allows to pause it, then you have an option to stop it. And I guess the difference is that stopping a cleaning - resets current cleaning status, maybe lets you to do a spot clean or another separate cleaning process... |
|
@Luc3as no need to change anything in homeassistant, when this PR (and other fixes) gets merged at some point, homeassistant component will just need a version bump. @Arnasnn that error with edit: sorry, I oversaw your update (that it works after |
|
The stop comment wasn't related to errors. Status error: Just looked into the app again, I don't see any way to upload any sound Stop issue: |
|
Ok. I think I added the @Luc3as you cannot push to repositories of other people, but you need to create your own fork (and then create a pull request afterwards), see https://help.github.com/articles/fork-a-repo/ |
syssi
approved these changes
Nov 16, 2018
|
Any idea when the next release is due ? |
|
Likely next weekend, if @syssi doesn't object or want to prepare an earlier release. |
|
Feel free! :-) |
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.
This seems to work fine with yeelight and gen1 vacuum,
however, more testing is needed with other devices and/or this change
should only be done for the vacuum.
Fixes #348, #364 and #370.