search

scottyphillips / pychonet

A simple to use library for interfacing with the ECHONETlite protocol
GNU General Public License v3.0
19 stars 17 forks source link

Unify pychonet interfaces #28

Closed klinkigt closed 2 years ago

klinkigt commented 3 years ago

During coding the light component of HA, I found that pychonet used different interfaces. With this I mean, sometimes you have the original HEX e.g. 0x12, sometimes without the 0x, but sending messages uses integer and in case of the light the hex/int are even replaced with the string:

values = {
   0x40: 'other',
   0x41: 'incandescent_lamp_color',
   0x42: 'white',
   0x43: 'daylight_white',
   0x43: 'daylight_color',
}

This made coding the HA component quite hacky. I need to send messages in integer, but the state array is populated with strings, but when I request some data it can also be hex. I think it is better to find a common interface.

The most logic for me would be, keep the lowest interfaces for getting data and sending messages in hex (just as echonetlite definition). than make some higher level interfaces using integer or even higher ones using strings. Then we can leave it to the developer of the down stream code to chose the interface they like. Now they need to dance around the API and need to know all those hex/interger things of echnonet anyway.

scottyphillips commented 3 years ago

Well, its hacky because it was put together for my own purposes to run my air conditioner in Home Assistant.

Those values above can be easily moved into the pychonet library under the specific GeneralLighting class - in fact once you have finished with the lighting code I will be rebuilding in accordance with all the other entities.

The design philosophy was guided by that purpose and that purpose alone not for any other reason. I built the original code over the space of 2 days.

The state arrays are populated with strings for values which have a function defined against the specific class. It was done that way because I am more comfortable with dealing with plain English and RESTFUL APIs, which in some way the library was intented to model.

There is always the generic EchonetInstance, which should in most cases return the raw value when running update() or getMessage(). getMessage() should ALWAYS return the raw value. See the examples below.

I think the only value where this rule does not apply is device.update(0x80) will return the strings ‘On’ or ‘Off’. That can possibly be reviewed. See below for the difference between the superclass and the subclass. 

from aioudp import UDPServer
from pychonet import Factory
from pychonet import ECHONETAPIClient as api
from pychonet import HomeAirConditioner
from pychonet import EchonetInstance

udp = UDPServer()
loop = asyncio.get_event_loop()
udp.run("0.0.0.0",3610, loop=loop)
server = api(server=udp,loop=loop)
await server.discover('192.168.1.6')
await server.getAllPropertyMaps('192.168.1.6',1,48,1)
device = EchonetInstance("192.168.1.6",1,48,1, server)
await device.getMessage(0x80)
b’0’

await device.update(0xA0)
‘32’

device2 = HomeAirConditioner("192.168.1.6", server)
await device2.getMessage(0x80)
b'0'

await device2.update(0xA0)
'low'

This isn’t the first time this idea has been discussed, but I am not particularly keen to change how this is being done. If you want to work in hex or int then use the generic EchonetIntance superclass.

If you want a more RESTFUL API type of experience use the more specific classes and enjoy polymorphism and more user friendly getters and setters.

klinkigt commented 3 years ago

Hi,

thank you for your reply. For me it is as well a hobby project, doing everything until it works. I understand your philosophy and agree with you. Also my code is ugly but works as good as I can make echonetlite work. Everything is setup automatically so there is not much to configure in HA side. Which is very nice I think. Please adjust my code to your liking. I will not touch it as for now. It came to this "in-between" state, as you just had commit a stub for the general lighting, while I was on the way to share my code.

I am also not sure which is the better API at which layer. Here I leave it up to you to make a strong statement and decide in which direction to go. After things are somehow settled, I may revisit the general lighting class again and see which after function could be implemented that are useful in HA. I suppose some of those transition function might be nice, but also have to look into it and understand echonet.

Thank again for all the efforts.

PS: Also thank for merging.

scottyphillips commented 3 years ago

Your light entity works well against the ECHONETLite simulation software that I have been using to redesign this custom component. I really want to thank you for your contribution.

klinkigt commented 3 years ago

Nothing to thank. You are the one we all need to thank. It is a bit of a love/hate relationship with me and echonetlite. I do not really want to use it, because it is such a complicated thing, but have no choice being in Japan. If things work out well, then I am fine. My main motivation was to get this upstream, so also my HA updates will go bit smoother now (at least I hope so :)).