larsbeck / HomematicIp

This package allows to query the HomematicIp REST and WebSocket endpoint.
MIT License
7 stars 2 forks source link

Implement teaching in and rename devices #16

Closed DevEddy closed 5 years ago

DevEddy commented 5 years ago

Hi Lars,

I was able to make some progress on teaching in devices and rename them. Also I added PluggableSwitch.

Later I will implement the possibility to assign the device to a room / group etc. like in the Homematic IP App.

Let me know what you think.

-- Eddy

larsbeck commented 5 years ago

Hi Eddy!

Excellent, thanks! I have merged your changes, looks good! :-)

Keep em coming! 👍

Lars

P.S.: Will you use "RequestInclusionRequestedCompletion()"? Otherwise we could remove that method and the field.

DevEddy commented 5 years ago

Hi Lars,

thank you!

Yeah, I wanted to simplify the teaching in process without to explicitly subscribe to events. It looks like this:

// request to teach in a device
await homematicService.StartDeviceInclusionProcess();

// waiting for the response when the teach in button of the device is pressed
var inclusionCompletion = homematicService.RequestInclusionRequestedCompletion();
var requestedForDeviceId = await inclusionCompletion.Task;

// The Homematic IP App prompts the user to enter the last 4 digits of the SGTIN
// here we just skip and use id of the event
await homematicService.StartInclusionModeForDevice(requestedForDeviceId);

Maybe you see a better way to do it?

-- Eddy

larsbeck commented 5 years ago

Hi Eddy,

I get the idea now. At least in theory I see a "race condition". This will probably never happen to anyone, but I think this shows how we could simplify even further. After

// request to teach in a device
await homematicService.StartDeviceInclusionProcess();

one could really quickly press the button on the device, when the following hasn't even been reached yet:

// waiting for the response when the teach in button of the device is pressed
var inclusionCompletion = homematicService.RequestInclusionRequestedCompletion();
var requestedForDeviceId = await inclusionCompletion.Task;

Now, in real life this will not happen, but we could integrate everything (all three methods) into one method that can be awaited. What do you think? If you think that is a good idea, I can make the changes and you check if they work for you.

DevEddy commented 5 years ago

Hi Lars,

you are right. I was thinking about that we maybe should implement it like in the original app. But we can skip the user input.

Would be nice if you could make the changes. I will test it.

-- Eddy

larsbeck commented 5 years ago

Hi Eddy,

I have merged everything into StartDeviceInclusionProcess. You can check out the changes in the latest commit.

Lars