alexmohr / sonyapilib

Contains a python api to control sony devices.
MIT License
20 stars 11 forks source link

Is an integration in upstream still pursued? #4

Open dilruacs opened 5 years ago

dilruacs commented 5 years ago

Hi Alex,

I own a BDP-S4200 and would like to add that to my home-assistant setup. For that, I would like to know if you still plan to integrate this component in upstream and if you need help testing.

The media-player components have been restructured a bit lately.

Regards, Edwin

alexmohr commented 5 years ago

The component for home assistant can be found here. https://github.com/alexmohr/home-assistant/blob/dev/homeassistant/components/media_player/sony.py

I never did the work to upstream the component because I only tested this library together with the bluray player I own which is not enough to ensure to high quality in home assistant. I'm using this component not very much myself. If you have an overview what have been changed in the media player it would be great if you could review the component and test it in your setup. Getting this library to run on more devices would enale the upstreaming of the component.

dilruacs commented 5 years ago

I know the home assistant component can be found elsewhere, I couldn't open an issue there.

I would offer to use your source (sony.py from your fork) to create a custom component, that can be installed under <config_dir>/custom_components. That would be less overhead as compared to a complete fork of home assistant.

If the custom component is adapted (needs minor adjustments) to the new media player component structure, we should document the models we own and try to find more Sony users on https://community.home-assistant.io/

As the sonyapilib apparently supports a whole range of Sony products (including TV sets, Streaming Players and Receivers), this component could possibly replace the braviatv component as well and maybe even become a generic Sony component.

alexmohr commented 5 years ago

To replace braviatv #2 must be solved because these devices use a newer api version. At the moment only api version 3 is tested. I currently have access to a TV which uses this api. I will take a look at finishing the implementation

The fork of home-assistant is necessary if we want to upstream the component. For testing purposes installing the component as custom is the way to go.

It sounds like you are up to date regarding the media-player interface. I'd appreciate it if you could adjust the component.

dilruacs commented 5 years ago

OK, as you wrote the API code, I'll trust you on the braviatv point. Replacing other components or becoming a generic component is a (very) distant possibility.

I'll create a custom component which can then be tested. If it is good enough to be merged upstream, one can copy these files into a fork of the dev branch easily and do a pull request.

dilruacs commented 5 years ago

You can find the slightly modified component here:

https://github.com/dilruacs/home-assistant-custom-components/tree/master/custom_components/sony

dilruacs commented 5 years ago

Now that I solved the pairing Issue in #5 ;-), I continued testing the Home Assistant component.

The changes needed for the new structure are:

+import voluptuous as vol

 from homeassistant.components.media_player import (
+    MediaPlayerDevice, PLATFORM_SCHEMA)
+from homeassistant.components.media_player.const import (
     SUPPORT_NEXT_TRACK, SUPPORT_PAUSE, SUPPORT_PREVIOUS_TRACK, SUPPORT_TURN_ON,
-    SUPPORT_TURN_OFF, SUPPORT_PLAY,SUPPORT_PLAY_MEDIA, SUPPORT_STOP,
-    MediaPlayerDevice)
-from homeassistant.const import (CONF_HOST, CONF_NAME, STATE_OFF, STATE_ON, STATE_PLAYING,
-    STATE_PAUSED)
+    SUPPORT_TURN_OFF, SUPPORT_PLAY,SUPPORT_PLAY_MEDIA, SUPPORT_STOP)
+from homeassistant.const import (CONF_HOST, CONF_NAME, STATE_OFF, STATE_ON,
+    STATE_PLAYING, STATE_PAUSED)
+import homeassistant.helpers.config_validation as cv

[...]

+PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({
+    vol.Required(CONF_HOST): cv.string,
+    vol.Required(CONF_NAME, default=DEFAULT_NAME): cv.string,
+})

After that I found an issue which prevented me from pairing my device using this component:

        # devices below version 3 do not require a pin.
        if auth_mode > 3:
            authenticated = sony_device.send_authentication(pin)

You check for auth_mode > 3 and skip pin authentication if true. My device has auth_mode == 3 and needs a pin. Did you read a spec somewhere, where an auth_mode > 3 wouldn't require a pin?

alexmohr commented 5 years ago

The check should be for auth_mode < 3 because API version 1 and 2 do not require a pin 3 and 4 do. I've mistakenly confused < and >. I could not find the specification for the API so it is mostly from KHerron's project and in some parts from the android app.

dilruacs commented 5 years ago

In that case it should say:

        # devices below version 3 do not require a pin.
        if auth_mode > 2:
            authenticated = sony_device.send_authentication(pin)
alexmohr commented 5 years ago

I linked your component in the readme markdown file so it can be found easily

alexmohr commented 5 years ago

I am nearly done with the implementation of the V4 api and just need some files from a v4 device which I can acquire on the weekend. After that It will probably take another week or so to finish the implementation.

Is there anything you need for the integration in home assistant which currently is not supported?

dilruacs commented 5 years ago

Yes. I do not have access to a v4 device, but I understand there is the need for a PSK. How exactly does this work? Does the enduser think of a PSK himself, which he enters in the TV/Media device and in the client configuration?

alexmohr commented 5 years ago

There is no need for the PSK. It is just an alternative to the pin authentication. I believe it is an alphanumeric value.

alexmohr commented 5 years ago

As the refactorings are done and the api version 4 has been implemented I'd be happy to receive feedback if it is still working with your device(s). When you don't discover any issues I'll release version 0.4 and we can go ahead and look for testers in the home assistant community.

Thanks again for your continued support

dilruacs commented 5 years ago

I will be on vacation for a week next week and my time is limited this week. I will update my version as soon as I am back from vacation and will let you know about issues I find as usual ;-) Thanks for the update!