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

Improve MiotDevice API (get_property_by, set_property_by, call_action, call_action_by) #905

Merged
merged 6 commits into from
Mar 8, 2021

Conversation

rytilahti
Copy link
Owner

@rytilahti rytilahti commented Jan 7, 2021

  • get_property_by(siid, piid) returns a single property, useful for testing with miiocli

  • set_property_by(siid, piid, value, value_type) allows setting one, value_type is optional and will be used for casting to correct type before passing the value to send().

  • call_action(name, params) calls an action with the given parameters, anything inside mapping that has aiid is considered an action.

  • call_action_by(siid, aiid, params) allows calling arbitrary actions, useful especially for command-line use and testing.

  • Also, code cleanup for passing the mapping.

  • Now the mapping is a class attribute, avoiding unnecessary init overloading

Testers wanted as I have no test devices!

Related to #901 (no more raw-commands necessary for the simplest test cases).

@rytilahti rytilahti changed the title WIP: Improve MiotDevice API (get_property_by, set_property_by commands) WIP: Improve MiotDevice API (get_property_by, set_property_by, call_action, call_action_by) Jan 16, 2021
@gudvinr
Copy link

gudvinr commented Jan 17, 2021

I tested these options with mmgg.pet_waterer.s1 which I have on hand.

  • get_property_by works
  • set_property_by works, but I only tried bool properties since this device doesn't really have many
  • call_action_by works too, but I tried single action which has no inputs and outputs

@rytilahti rytilahti force-pushed the feat/improve_miotdevice branch from 2050091 to a09cd65 Compare February 12, 2021 22:50
@rytilahti rytilahti changed the title WIP: Improve MiotDevice API (get_property_by, set_property_by, call_action, call_action_by) Improve MiotDevice API (get_property_by, set_property_by, call_action, call_action_by) Feb 12, 2021
@rytilahti
Copy link
Owner Author

@arturdobo @darckly @bafonins if you don't mind testing this PR on your miot devices, it would be great! The conversion from using passed _MAPPING to a mapping defined inside the implementing class may have some glitches.

@darckly
Copy link
Contributor

darckly commented Feb 14, 2021

I testes options with my Huizuo lamps - get_properties_by and set_properties_by are working as expected, the fourth parameter (value_type) should be always specified.
Checked with power (bool), brightness (int), and temperature (int).

call_action and call_action_by methods I can't test

@arturdobo
Copy link
Contributor

Sorry for such a late response. I was able to call status on

  • zhimi.airpurifier.mb4
  • zhimi.airpurifier.m2
  • zhimi.humidifier.ca4
  • zhimi.airpurifier.v7
  • zhimi.airpurifier.mb3

* Also, code cleanup for passing the mapping.
* Now the mapping is a class attribute, avoiding unnecessary __init__ overloading
Also, fix passing booleans to set_property_by
@rytilahti rytilahti merged commit 7c0a5a5 into master Mar 8, 2021
@rytilahti rytilahti deleted the feat/improve_miotdevice branch March 8, 2021 13:17
@rytilahti rytilahti mentioned this pull request Mar 13, 2021
) -> None:
self.mapping = mapping
super().__init__(ip, token, start_id, debug, lazy_discover)
mapping = None
Copy link
Contributor

Choose a reason for hiding this comment

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

Hello,
This seems to be a breaking change for MiotDevice. We can no longer use

MiotDevice(ip=host, token=token, mapping=mapping)

to instantiate a MiotDevice with a given mapping (ha0y/xiaomi_miot_raw#79).
Is there a method to keep backward compatibility?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Hi, sorry about that! You could assign the mapping directly after constructing the MiotDevice object to fix this for now:

dev = MiotDevice(...)
dev.mapping = my_mapping

Otherwise, feel free to open an issue to re-add the kwarg variant of this parameter back to the constructor!

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for your prompt reply!

Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunatelly, this doesn't solve the problem, as this code doesn't work in version 0.5.4. mapping is required in 0.5.4 but unexpected in 0.5.5. I have opened an issue #982. Hope this will be fixed soon because it is a breaking change and may affect those who uses this library.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Indeed, I hadn't thought about that, sorry... See my comment on the issue how I plan to rectify it.

xvlady pushed a commit to xvlady/python-miio that referenced this pull request May 9, 2021
…, call_action_by) (rytilahti#905)

* Improve MiotDevice API (get_property_by, set_property_by commands)

* Also, code cleanup for passing the mapping.
* Now the mapping is a class attribute, avoiding unnecessary __init__ overloading

* Add call_action(name,params) and call_action_by(siid,aiid,params)

Also, fix passing booleans to set_property_by

* Add some unittests

* Convert a couple of missed/new classes to use new mapping

* Fix linting

* make vacuum tests pass
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.

5 participants