Add inital support for mmgg.feeder.petfeeder #1210

Wh1terat · 2021-12-06T03:36:57Z

I've tried to follow the style as best I can see.
Was 50/50 on calling the integration "PetfoodDispenser" (keeps with PetWaterDispenser) or simply calling it "PetFeeder".

This device does not follow it's own (somewhat limited) miot spec (annoying since the Manufacturer does make other compliant devices!) so this is all MiiO.

There's a number of features unavailable vs the mi home app:

Fault status (seemingly only available via push to mihome)
Feed total (seemingly stored in the plugin within mihome)

Added most functions:

Usage: miiocli petfooddispenser [OPTIONS] COMMAND [ARGS]...

Options:
  --ip TEXT     [required]
  --token TEXT  [required]
  --model TEXT
  --help        Show this message and exit.

Commands:
  dispense_food     Dispense food.
  info              Get (and cache) miIO protocol information from the...
  raw_command       Send a raw command to the device.
  reset_clean_time  Reset clean time.
  reset_dryer_time  Reset dryer time.
  set_feed_state    Enable / Disable the automatic feeding schedule.
  set_key_lock      Enable / Disable the key lock for the manual dispense...
  set_wifi_led      Enable / Disable the wifi status led.
  status            Retrieve properties.
  test_properties   Helper to test device properties.

Usage: miiocli petfooddispenser dispense_food [OPTIONS] AMOUNT

  Dispense food. :param amount: in units (1 unit ~= 5g)

Usage: miiocli petfooddispenser reset_clean_time [OPTIONS]

  Reset clean time.

Usage: miiocli petfooddispenser reset_dryer_time [OPTIONS]

  Reset dryer time.

Usage: miiocli petfooddispenser set_feed_state [OPTIONS] STATE

  Enable / Disable the automatic feeding schedule.

Usage: miiocli petfooddispenser set_key_lock [OPTIONS] STATE

  Enable / Disable the key lock for the manual dispense button.

Usage: miiocli petfooddispenser set_wifi_led [OPTIONS] STATE

  Enable / Disable the wifi status led.

Will look into adding support for feedplans soon.

Feedback welcome.

rytilahti

A very brief review, looking great! Please do not forget to add the device to README.md, too

miio/integrations/petfooddispenser/status.py

miio/integrations/petfooddispenser/device.py

Co-authored-by: Teemu R. <[email protected]>

Previously threw NotImplementedError, but have removed all together now.

Wh1terat · 2021-12-06T16:07:53Z

Think that's everything fixed 😸

rytilahti · 2021-12-06T16:25:20Z

miio/integrations/petfooddispenser/device.py

+            "Power source: {result.power_status}\n"
+            "Food level: {result.food_status}\n"
+            "Automatic feeding: {result.feed_plan}\n"
+            "Food bin lid: {result.door_status}\n"
+            "Dispense button lock: {result.key_lock}\n"
+            "Days until clean: {result.clean_days}\n"
+            "Desiccant life: {result.dryer_days}\n"
+            "WiFi LED: {result.wifi_led}\n",


It would make sense to avoid using the property names from the protocol itself, but use more developer friendly names (similar to use for these printouts).

Ah yeah I was going to ask about that.

Some names I've taken from miot spec for another feeder by mmgg (one that's actually compliant... 😠 ), but obviously some things don't translate overly well and wasn't sure on the preferred approach.

So you're suggesting (for example) "door_status" > "food_bin_lid" or something along those lines?

Yes, exactly. It makes sense to separate the property names from what is being internally used, as most of the times they are not clear in their meaning. bin_lid_open, maybe?

miio/integrations/petfooddispenser/device.py

rytilahti · 2021-12-06T16:27:26Z

miio/integrations/petfooddispenser/device.py

+        return self.send("resetdryer")
+
+    @command(
+        click.argument("state", type=int),


Suggested change

click.argument("state", type=int),

click.argument("state", type=bool),

Use bool for boolean values across the board. This should work directly for the cli input values so no need for explicit conversions either.

I used bool initially but seemingly then gets sent as literal "true" or "false" (not 1 or 0) which the device does not understand. miot spec for other mmgg products have these defined as ints rather than bools.

Oh, then casting is needed; int(boolvalue) will fix that.

🤦
Given I wrote it at 3am I'm amazed there's not more glaring issues. I'll sort that.

miio/integrations/petfooddispenser/status.py

rytilahti · 2021-12-06T16:30:03Z

miio/integrations/petfooddispenser/device.py

+        return self.send("outfood", [amount])
+
+    @command(default_output=format_output("Resetting clean time"))
+    def reset_clean_time(self) -> bool:


Are you sure this returns a bool? And same for reset_dryer_time?

Good catch, it returns a list with a string of "ok".
That said, it returns "ok" literally no matter what you send to this device 🤦

That's common on commands, coming likely from the SDK. You could either check for the "ok" in the payload or simply return it as it is, there is no real consistency in the library at the moment on that.

Co-authored-by: Teemu R. <[email protected]>

rytilahti · 2021-12-06T16:37:52Z

miio/integrations/petfooddispenser/status.py

+        return bool(self.data["door_status"])
+
+    @property
+    def clean_days(self) -> int:


Use timedelta for this and the dryer one, this makes it consistent with other integrations.
Also, use naming scheme <variable>_left.

Perfect, was just about to ask about this.
"dryer_days" > "desiccant_left"

Can't quite think how to phrase "clean_days".
It's the number of days until the unit requires cleaning.
Suggestions?

I don't have a good term for that either, time_until_clean_left is sounds quite verbose.

Yeah that's what I was trying to avoid, even though it's very pythonic to make it as long as is needed to convey it's use.

rytilahti · 2021-12-06T16:39:47Z

miio/integrations/petfooddispenser/device.py

+            lambda state: "Enabling automatic feeding schedule" if state else "Disabling automatic feeding schedule"
+        ),
+    )
+    def set_feed_state(self, state: int):


set_automated_feeding <-> automated_feeding in state, maybe?

Naming stuff is the hardest part of all this, honestly 😆

In mihome the toggle is "automatic feeding schedule".

Btw, you can run the linting tests locally by executing tox -e lint or pre-commit run -a (or alternatively, you can do pre-commit install and it will perform the checks prior commiting).

I must admit I didn't with this commit, I will in future 👼

Wh1terat · 2021-12-06T18:41:31Z

Infact yeah, although this was an "initial support" pull - give me a day or so and I'll just refactor and add everything and make it fully supported from day one.

Wh1terat · 2021-12-11T15:24:15Z

Work got a bit busy (chaos before Christmas change freeze!) but have started.

Few questions before we have another shot ;)

1. Implementing field maximums (i.e max dispense unit is 30)
Should this just be a logged error and return false from the relevant function or should I raise an exception? And if exception is it ok to use standard exceptions (i.e ValueError ) or should I subclass DeviceException (as I have seen in other modules).

2. Timezones!
I've implemented the feedplan schedules now. The device stores these as CST timezone, which isn't a problem for displaying as I've just made them into datetime objects with the appropriate tzinfo. (Bit annoying that datetime.time doesn't support astimezone, so having to make them full datetime.datetime objects with 1970,1,1 date 😠 )
But the big question, for adding a feedplan - what format would you suggest for the user to input this towards my function?

datetime.time with tzinfo?
datetime.datetime with tzinfo?
Hour, min as int and iana tz string?

3. Boolean responses
What's the general approach for boolean responses, just to return truthy falsy or to map these to string values?
Or create enums just for the 2 choices?
e.g:
On/Off
Locked/Unlocked
Open/Closed
Enabled/Disabled

rytilahti · 2021-12-13T00:53:17Z

Should this just be a logged error and return false from the relevant function or should I raise an exception? And if exception is it ok to use standard exceptions (i.e ValueError ) or should I subclass DeviceException (as I have seen in other modules).

Good catch, please use standard exceptions. I modified #1114 to list it as something that should be consolidated across the code base.

What's the general approach for boolean responses, just to return truthy falsy or to map these to string values?

Use booleans where possible, no need to enumize.

On the topic of timezones, I'm not sure what's the best approach here. IIRC the vacuum schedules do convert the times to the local tz, maybe take a look how it's done and follow the same approach?

Wh1terat · 2021-12-15T04:26:25Z

Closing in favour of #1253

Add inital support for mmgg.feeder.petfeeder

56cd67d

Wh1terat mentioned this pull request Dec 6, 2021

Xiaomi Xiaowan Smart pet feeder mmgg.feeder.petfeeder #1047

Open

rytilahti requested changes Dec 6, 2021

View reviewed changes

Wh1terat and others added 4 commits December 6, 2021 15:53

Update miio/integrations/petfooddispenser/status.py

4672972

Co-authored-by: Teemu R. <[email protected]>

Update miio/integrations/petfooddispenser/device.py

d3f538d

Co-authored-by: Teemu R. <[email protected]>

Update miio/integrations/petfooddispenser/device.py

ddb6281

Co-authored-by: Teemu R. <[email protected]>

Add example data

520a57a

rytilahti added miio new device labels Dec 6, 2021

Wh1terat added 3 commits December 6, 2021 16:00

Fix typo (desiccant)

d14b8ed

Removed feed_today and food_portion properties

29965e7

Previously threw NotImplementedError, but have removed all together now.

Remove county_code property

8a3b2a5

Update README.rst

9b92a09

rytilahti reviewed Dec 6, 2021

View reviewed changes

Update miio/integrations/petfooddispenser/status.py

9495768

Co-authored-by: Teemu R. <[email protected]>

rytilahti reviewed Dec 6, 2021

View reviewed changes

Merge branch 'rytilahti:master' into petfeeder-1

cebf8e8

Wh1terat closed this Dec 15, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add inital support for mmgg.feeder.petfeeder #1210

Add inital support for mmgg.feeder.petfeeder #1210

Wh1terat commented Dec 6, 2021

rytilahti left a comment

Wh1terat commented Dec 6, 2021

rytilahti Dec 6, 2021

Wh1terat Dec 6, 2021

rytilahti Dec 6, 2021 •

edited

Loading

rytilahti Dec 6, 2021

Wh1terat Dec 6, 2021

rytilahti Dec 6, 2021

Wh1terat Dec 6, 2021

rytilahti Dec 6, 2021

Wh1terat Dec 6, 2021

rytilahti Dec 6, 2021

rytilahti Dec 6, 2021

Wh1terat Dec 6, 2021

rytilahti Dec 6, 2021

Wh1terat Dec 6, 2021

rytilahti Dec 6, 2021

Wh1terat Dec 6, 2021

rytilahti Dec 6, 2021

Wh1terat Dec 6, 2021

Wh1terat commented Dec 6, 2021

Wh1terat commented Dec 11, 2021

rytilahti commented Dec 13, 2021

Wh1terat commented Dec 15, 2021

	click.argument("state", type=int),
	click.argument("state", type=bool),

Add inital support for mmgg.feeder.petfeeder #1210

Add inital support for mmgg.feeder.petfeeder #1210

Conversation

Wh1terat commented Dec 6, 2021

rytilahti left a comment

Choose a reason for hiding this comment

Wh1terat commented Dec 6, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rytilahti Dec 6, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Wh1terat commented Dec 6, 2021

Wh1terat commented Dec 11, 2021

rytilahti commented Dec 13, 2021

Wh1terat commented Dec 15, 2021

rytilahti Dec 6, 2021 •

edited

Loading