rgerganov / py-air-control

Command line app for controlling Philips air purifiers
MIT License
256 stars 52 forks source link

Cleanup of airctrl.py #81

Closed Cyber1000 closed 3 years ago

Cyber1000 commented 3 years ago

tl;dr

I did a bigger refactoring last year and added the coap-part, but changes in airctrl.py where only a first approach. There where differences in output and many redundances between the 3 protocols (http, plaincoap and coap) This PR cleans airctrl.py - output will be the same no matter which protocol you use

longer information, points to pay attention

changes to cli:

rgerganov commented 3 years ago

Thank you for the PR. I will try to review and provide feedback soon.

rgerganov commented 3 years ago

Sorry for taking me so long to get to this. I have reviewed and tested your changes, everything looks fine to me! About the behavior change for HTTP devices -- I prefer to get the updated values after changing settings but I am also fine with your approach. I think the best way to switch between those two behaviors is at the end of the main function:

        if values:
            c.set_values(values, debug=args.debug)
        else:
            c.get_status(debug=args.debug)

vs.

        if values:
            c.set_values(values, debug=args.debug)
        c.get_status(debug=args.debug)
Cyber1000 commented 3 years ago

No problem, was busy too. Well yes I think the proposed change at the end of the main function would be the best position if we want to add the get after a set. Maybe in another PR. Thanks for merging!