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

Add support for Walkingpad A1 (ksmb.walkingpad.v3) #975

Merged
merged 15 commits into from Apr 9, 2021
Merged

Add support for Walkingpad A1 (ksmb.walkingpad.v3) #975

merged 15 commits into from Apr 9, 2021

Conversation

dewgenenny
Copy link
Contributor

@dewgenenny dewgenenny commented Mar 17, 2021

Hey there, I had a go at adding support for the Walkingpad A1.

What's working:

status, turn on/off, start/stop and set_speed.

What's not working:

Full testing for the status command. The Walkingpad A1 allows retrieval of only 1 property at a time. This means a full status update takes quite a long time to answer. Luckily there is an 'all' property that returns the majority of the status elements. However, this makes testing of the status and speed updates really difficult. (I am sure it's possible but it is outside of my current abilities).

Hope this is good enough to merge... if not be gentle... it's my first ever PR ;)

Fixes #797

@dewgenenny dewgenenny mentioned this pull request Mar 17, 2021
Copy link
Owner

@rytilahti rytilahti left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I did a cursory code review, but it is looking quite good already :-)

miio/walkingpad.py Outdated Show resolved Hide resolved
miio/walkingpad.py Outdated Show resolved Hide resolved
miio/walkingpad.py Outdated Show resolved Hide resolved
miio/walkingpad.py Outdated Show resolved Hide resolved
miio/walkingpad.py Outdated Show resolved Hide resolved
miio/walkingpad.py Outdated Show resolved Hide resolved
miio/walkingpad.py Outdated Show resolved Hide resolved
miio/walkingpad.py Outdated Show resolved Hide resolved
@dewgenenny
Copy link
Contributor Author

Thanks man for the really quick response and review! Will make the changes later today hopefully :)

Copy link
Owner

@rytilahti rytilahti left a comment

Choose a reason for hiding this comment

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

Sure thing! I just happened to be there and thought I can give a very quick review as it didn't look too complicated :-) Here's another round of reviews, I'll review the tests afterwards.

miio/walkingpad.py Outdated Show resolved Hide resolved
miio/walkingpad.py Outdated Show resolved Hide resolved
miio/walkingpad.py Outdated Show resolved Hide resolved
miio/walkingpad.py Outdated Show resolved Hide resolved
miio/walkingpad.py Outdated Show resolved Hide resolved
miio/walkingpad.py Outdated Show resolved Hide resolved
miio/walkingpad.py Outdated Show resolved Hide resolved
miio/walkingpad.py Show resolved Hide resolved
miio/walkingpad.py Outdated Show resolved Hide resolved
miio/walkingpad.py Outdated Show resolved Hide resolved
@dewgenenny
Copy link
Contributor Author

dewgenenny commented Mar 30, 2021

Hey @rytilahti just pushed a new version. Contains all your requested changes and hopefully is closer to something that makes sense to merge :p

@dewgenenny
Copy link
Contributor Author

not sure if I should have 'requested new review' - TOTALLY didnt want to be pushy, just not sure what the normal flow is ;)

appreciate any and all time and help man!

Copy link
Owner

@rytilahti rytilahti left a comment

Choose a reason for hiding this comment

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

Hi @dewgenenny, no worries about pinging for a review, it helps to give a notification that the PR is ready to be reviewed even when I'm receiving updates to PRs (which may or may not imply that it's ready).

I added some more comments, could you also update README.md to include the device?

miio/walkingpad.py Outdated Show resolved Hide resolved
miio/walkingpad.py Outdated Show resolved Hide resolved
return self.power == "on"

@property
def time(self) -> int:
Copy link
Owner

Choose a reason for hiding this comment

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

What do you think about this?

miio/walkingpad.py Show resolved Hide resolved
miio/walkingpad.py Outdated Show resolved Hide resolved
@dewgenenny
Copy link
Contributor Author

OK man! Just finished updating as per your comments. I am doing this all while walking on the treadmill, so I am working fast so I can sit down soon 😆

miio/walkingpad.py Show resolved Hide resolved
miio/walkingpad.py Outdated Show resolved Hide resolved
miio/walkingpad.py Outdated Show resolved Hide resolved
miio/walkingpad.py Outdated Show resolved Hide resolved
miio/walkingpad.py Outdated Show resolved Hide resolved
miio/walkingpad.py Outdated Show resolved Hide resolved
miio/walkingpad.py Outdated Show resolved Hide resolved
@dewgenenny
Copy link
Contributor Author

Next round done :) thanks for all your patience and guidance man! Some of the checks in the build pipeline were not successful, but I think this is more to do with pipeline stability thank something specific in my code. I would hit re-run but don't have the necessary permissions

Copy link
Owner

@rytilahti rytilahti left a comment

Choose a reason for hiding this comment

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

I restarted the CI and looks like everything is fine on that front. Here just a couple minor nitpicks and this is good to go, thanks for your patience!

miio/walkingpad.py Outdated Show resolved Hide resolved
miio/walkingpad.py Outdated Show resolved Hide resolved
miio/walkingpad.py Outdated Show resolved Hide resolved
@dewgenenny
Copy link
Contributor Author

Added the changes you requested.... really really appreciate the time and patience you have taken with me to get this done!

@rytilahti
Copy link
Owner

Looks good to go, thanks @dewgenenny! 🎉

@rytilahti rytilahti changed the title Initial integration walkingpad Add support for Walkingpad A1 (ksmb.walkingpad.v3) Apr 9, 2021
@rytilahti rytilahti merged commit a9072a2 into rytilahti:master Apr 9, 2021
rytilahti added a commit that referenced this pull request May 5, 2021
[Full Changelog](0.5.5.2...0.5.6)

**Implemented enhancements:**

- RFC: Add a script to simplify finding supported properties for miio [\#919](#919)
- Improve test\_properties output [\#1024](#1024) ([rytilahti](https://github.com/rytilahti))
- Relax zeroconf version requirement [\#1023](#1023) ([rytilahti](https://github.com/rytilahti))
- Add test\_properties command to device class [\#1014](#1014) ([rytilahti](https://github.com/rytilahti))
- Add discover command to miiocli [\#1013](#1013) ([rytilahti](https://github.com/rytilahti))
- Fix supported oscillation angles of the dmaker.fan.p9 [\#1011](#1011) ([syssi](https://github.com/syssi))
- Add additional operation mode of the deerma.humidifier.jsq1 [\#1010](#1010) ([syssi](https://github.com/syssi))
- Roborock S7: Parse history details returned as dict [\#1006](#1006) ([fettlaus](https://github.com/fettlaus))

**Fixed bugs:**

- zeroconf 0.29.0 which is incompatible [\#1022](#1022)
- Remove superfluous decryption failure for handshake responses [\#1008](#1008)
- Skip pausing on Roborock S50 [\#1005](#1005)
- Roborock S7 after Firmware Update 4.1.2-0928 - KeyError [\#1004](#1004)
- No air quality value when aqi is 1 [\#958](#958)
- Fix exception on devices with removed lan\_ctrl [\#1028](#1028) ([Kirmas](https://github.com/Kirmas))
- Fix start bug and improve error handling in walkingpad integration [\#1017](#1017) ([dewgenenny](https://github.com/dewgenenny))
- gateway: fix zigbee lights [\#1016](#1016) ([starkillerOG](https://github.com/starkillerOG))
- Silence unable to decrypt warning for handshake responses [\#1015](#1015) ([rytilahti](https://github.com/rytilahti))
- Fix set\_mode\_and\_speed mode for airdog airpurifier [\#993](#993) ([alexeypetrenko](https://github.com/alexeypetrenko))

**Closed issues:**

- Add Dafang camera \(isa.camera.df3\) support [\#996](#996)
- Roborock S7 [\#989](#989)
- WalkingPad A1 Pro [\#797](#797)

**Merged pull requests:**

- Add basic dmaker.fan.1c support [\#1012](#1012) ([syssi](https://github.com/syssi))
- Always return aqi value \[Revert PR\#930\] [\#1007](#1007) ([bieniu](https://github.com/bieniu))
- Added S6 to skip pause on docking [\#1002](#1002) ([Sian-Lee-SA](https://github.com/Sian-Lee-SA))
- Added number of dust collections to CleaningSummary if available [\#992](#992) ([fettlaus](https://github.com/fettlaus))
- Reformat history data if returned as a dict/Roborock S7 Support \(\#989\) [\#990](#990) ([fettlaus](https://github.com/fettlaus))
- Add support for Walkingpad A1 \(ksmb.walkingpad.v3\) [\#975](#975) ([dewgenenny](https://github.com/dewgenenny))
@rytilahti rytilahti mentioned this pull request May 5, 2021
rytilahti added a commit that referenced this pull request May 5, 2021
[Full Changelog](0.5.5.2...0.5.6)

**Implemented enhancements:**

- RFC: Add a script to simplify finding supported properties for miio [\#919](#919)
- Improve test\_properties output [\#1024](#1024) ([rytilahti](https://github.com/rytilahti))
- Relax zeroconf version requirement [\#1023](#1023) ([rytilahti](https://github.com/rytilahti))
- Add test\_properties command to device class [\#1014](#1014) ([rytilahti](https://github.com/rytilahti))
- Add discover command to miiocli [\#1013](#1013) ([rytilahti](https://github.com/rytilahti))
- Fix supported oscillation angles of the dmaker.fan.p9 [\#1011](#1011) ([syssi](https://github.com/syssi))
- Add additional operation mode of the deerma.humidifier.jsq1 [\#1010](#1010) ([syssi](https://github.com/syssi))
- Roborock S7: Parse history details returned as dict [\#1006](#1006) ([fettlaus](https://github.com/fettlaus))

**Fixed bugs:**

- zeroconf 0.29.0 which is incompatible [\#1022](#1022)
- Remove superfluous decryption failure for handshake responses [\#1008](#1008)
- Skip pausing on Roborock S50 [\#1005](#1005)
- Roborock S7 after Firmware Update 4.1.2-0928 - KeyError [\#1004](#1004)
- No air quality value when aqi is 1 [\#958](#958)
- Fix exception on devices with removed lan\_ctrl [\#1028](#1028) ([Kirmas](https://github.com/Kirmas))
- Fix start bug and improve error handling in walkingpad integration [\#1017](#1017) ([dewgenenny](https://github.com/dewgenenny))
- gateway: fix zigbee lights [\#1016](#1016) ([starkillerOG](https://github.com/starkillerOG))
- Silence unable to decrypt warning for handshake responses [\#1015](#1015) ([rytilahti](https://github.com/rytilahti))
- Fix set\_mode\_and\_speed mode for airdog airpurifier [\#993](#993) ([alexeypetrenko](https://github.com/alexeypetrenko))

**Closed issues:**

- Add Dafang camera \(isa.camera.df3\) support [\#996](#996)
- Roborock S7 [\#989](#989)
- WalkingPad A1 Pro [\#797](#797)

**Merged pull requests:**

- Add basic dmaker.fan.1c support [\#1012](#1012) ([syssi](https://github.com/syssi))
- Always return aqi value \[Revert PR\#930\] [\#1007](#1007) ([bieniu](https://github.com/bieniu))
- Added S6 to skip pause on docking [\#1002](#1002) ([Sian-Lee-SA](https://github.com/Sian-Lee-SA))
- Added number of dust collections to CleaningSummary if available [\#992](#992) ([fettlaus](https://github.com/fettlaus))
- Reformat history data if returned as a dict/Roborock S7 Support \(\#989\) [\#990](#990) ([fettlaus](https://github.com/fettlaus))
- Add support for Walkingpad A1 \(ksmb.walkingpad.v3\) [\#975](#975) ([dewgenenny](https://github.com/dewgenenny))
xvlady pushed a commit to xvlady/python-miio that referenced this pull request May 9, 2021
* Initial integration walkingpad

* Removed print statement

* Implement PR review suggestions

* Fix step_count bug

* Update miio/walkingpad.py

Co-authored-by: Teemu R. <tpr@iki.fi>

* Fix step_count bug

* Update based on PR feedback & add startup speed / sensitivity functions

* Update docstring with class initialisation

* Implement PR feedback

* Rename time to walking_time and change to return timedelta

* Rename time to walking_time and change to return timedelta

* Correct the description for starting & stopping

* Fix mode and sensitivity return types. Resolve more PR feedback

* Change start function to power-on if treadmill is off when called. Also other minor PR feedback changes.

Co-authored-by: Teemu R. <tpr@iki.fi>
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.

WalkingPad A1 Pro
2 participants