search

newAM / hisensetv

Python API to control Hisense brand TVs via their internal MQTT broker.
MIT License
37 stars 17 forks source link

Update __init__.py #3

Closed ejonesnospam closed 4 years ago

ejonesnospam commented 4 years ago

Adds media controls. Improves error control for send_key commands

newAM commented 4 years ago

I committed some changed to your branch - I narrowed the scope of the exception catching for JSON to just the JSONDecodeError, and removed the catch for _send_key exceptions.

I would prefer that _send_key propagate exceptions up to the user so that in case of problems that could be fixed by the user they get all the information instead of a True / False.

Let me know what you think!

Sorry for the slow replies, the coronavirus has made my life a little crazy recently.

ejonesnospam commented 4 years ago

No reason to apologize, if your community is not at or near chaos right now, you must be living in space.

I can totally agree understand that _send_key may should remain as is. After looking at again, I agree the exceptions are better if propagated up. To allow more functionality, would it be better to include a template public command in the api, like below? Since the MQTT interface is not really supported publicly by Hisense, any future firmware change could alter or add more commands, and a fix may not require an API change (if lucky). def send_key_template(self, keyname: str): """ Sends a keypress to the TV, as if it had been pressed on the IR remote. Args: keyname: Name of the key press to send. """ self._call_service(service="remote_service", action="sendkey", payload=keyname)

newAM commented 4 years ago

Nothing is every truly private in python so someone could just overwrite the private _send_key, maybe we should make it public?

class MyTv(HisenseTv):
    def _send_key(self, keyname: str):
        self._call_service(service="remote_service", action="new_sendkey_action", payload=keyname)
ejonesnospam commented 4 years ago

Yes, I feel it should be public. Also updated README for new media control commands. How do you like the nomenclature: "fast_forward" , "rewind", "volume_up" ?

newAM commented 4 years ago

Looks good to me! Should I merge now?

ejonesnospam commented 4 years ago

Yes, please. Stay safe

On Apr 5, 2020 at 11:06 AM, Alex notifications@github.com wrote:

Looks good to me! Should I merge now?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

ejonesnospam commented 4 years ago

Can you please update PyPi so I can download version 0.0.7