Skip to content
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

Added some parameters: Error code, Viomimode, Viomibintype #799

Merged
merged 3 commits into from
Nov 2, 2020
Merged

Added some parameters: Error code, Viomimode, Viomibintype #799

merged 3 commits into from
Nov 2, 2020

Conversation

fs79
Copy link
Contributor

@fs79 fs79 commented Aug 15, 2020

added:

error codes
2103: "Charging",
2105: "Fully charged"

saw the 4 but don´t know what it means, but so no error message in cli command
class ViomiMode(Enum):
Unknown = 4

class ViomiBinType(Enum):
NoBin = 0

added:

error codes
    2103: "Charging",
    2105: "Fully charged"

saw the 4 but don´t know what it means, but so no error message in cli command
class ViomiMode(Enum):
    Unknown = 4

class ViomiBinType(Enum):
    NoBin = 0
@coveralls
Copy link

Coverage Status

Coverage increased (+0.007%) to 73.978% when pulling 5a3f2f4 on fs79:master into db2cb9c on rytilahti:master.

@@ -63,7 +65,7 @@ class ViomiMode(Enum):
Vacuum = 0 # No Mop, Vacuum only
VacuumAndMop = 1
Mop = 2

Unknown = 4
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would make more sense to add a log message(s) to the place(s) where the unknown mode is encountered, otherwise this looks good to go, thanks for the PR! 👍

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ping?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand your comment, you mean a log message in the mode method ?
like here:

?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to say that it's more useful to simply log the unknown state on the call site, like done in

-- in case someone is seeing those warnings in their logs and knows what causes them, they can create a PR to fix it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If @fs79 doesn't answer, I can integrate this patch in this PR (#808)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mode 4 is something like CleanPosition.

@@ -38,6 +38,8 @@
530: "Mop and water tank missing",
531: "Water tank is not installed",
2101: "Unsufficient battery, continuing cleaning after recharge",
2103: "Charging",
2105: "Fully charged",
Copy link
Contributor

@titilambert titilambert Aug 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirm this message, I have the same one
Thanks !

@fs79
Copy link
Contributor Author

fs79 commented Sep 12, 2020

I‘m fine with logging the unknown state.
It’s important that the unknown state doesn’t destroy the output of the cli command.
Im using the output and if there’s an issue due to unknown parameters it‘s an issue.

Would you do it in general for unknown states in all the outputs or only for the above mentioned vacuum/mop/... state?

@rytilahti
Copy link
Owner

Yes, I think it makes sense to log all unknown cases to avoid potential exceptions. Btw, if you want to parse the command line output, you may be interested in checking out the --output flag.

@fs79
Copy link
Contributor Author

fs79 commented Sep 12, 2020

@rytilahti
You mean the "-o" flag?
Yes, I´m using it vor type vacuum but for viomivacuum it's not possible to use this flag.

@rytilahti
Copy link
Owner

Yeah, I think -o is the shortcut for that flag, which should definitely be available (and working) on all devices, so it must be a bug that can be fixed. Please feel free to create a separate issue / debug that / create a PR to fix the issue.

@fs79
Copy link
Contributor Author

fs79 commented Sep 13, 2020

Issue for the -o json_pretty issue with viomivacuum cleaner.
#816

@syssi syssi added this to the 0.5.4 milestone Nov 2, 2020
@rytilahti rytilahti merged commit 05da523 into rytilahti:master Nov 2, 2020
xvlady pushed a commit to xvlady/python-miio that referenced this pull request May 9, 2021
…#799)

* Error code, Viomimode, Viomibintype

added:

error codes
    2103: "Charging",
    2105: "Fully charged"

saw the 4 but don´t know what it means, but so no error message in cli command
class ViomiMode(Enum):
    Unknown = 4

class ViomiBinType(Enum):
    NoBin = 0

* Make operation modes complete

* Fix lint issue

Co-authored-by: Sebastian Muszynski <basti@linkt.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants