search

probonopd / ESP8266HueEmulator

Emulate a Philips Hue bridge running on an ESP8266 using the Arduino IDE.
MIT License
411 stars 93 forks source link

Alexa discovery updates #87

Closed Quanghoster closed 7 years ago

Quanghoster commented 7 years ago

I@ve just synchronised my fork and noticed that the Alexa discovery was still not working in the main repository. I've included the necessary changes to correct this, namely a new function that formats the returned json to alexa on discovery and corrected the case of one of the SSDP attributes, as observed on the network

dtila commented 7 years ago

Hi @Quanghoster . This pull request overrides mine https://github.com/probonopd/ESP8266HueEmulator/pull/85

If you revert your changes and merge from master, you will be able to see the hue by Alexa. I can say that since I had an Alexa meanwhile

Quanghoster commented 7 years ago

Hmm, strange. I thought I have brought.my fork up to date with your repository before I tested it with alexa and it didn't work for me last night. Github is confusing On 20 Oct 2017 08:51, Daniel notifications@github.com wrote:Hi @Quanghoster . This pull request overrides mine #85 If you revert your changes and merge from master, you will be able to see the hue by Alexa. I can say that since I had an Alexa meanwhile

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

dtila commented 7 years ago

You can see https://github.com/probonopd/ESP8266HueEmulator/pull/85/commits/6c63513c8bce384f40ca73789fb07dd681f49897#diff-e15aea686d057abe4787cdfd553060f8R1040 . Along with your suggestions, I added support for dimmable light.

Have a look at the link, and make a comparison please.

Quanghoster commented 7 years ago

Ok, I must have completed the fork sync incorrectly. I'm managed to get it in sync and it now works without modification. Thanks 👍

probonopd commented 7 years ago

So how should we proceed with this PR?

Quanghoster commented 7 years ago

We should cancel this one, however that is done. However, I don't think the code is quite right still. The master now does discover correctly but I don't think is correctly responding to the echo turn on and off requests,  which I had also fixed in my repository. I need to check the cause of that over the next couple of days, when I can fit it in. I think that may be the reason for the json format change I added in this PR On 21 Oct 2017 10:05, probonopd notifications@github.com wrote:So how should we proceed with this PR?

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

Quanghoster commented 7 years ago

If I find the issue I will post an updated PR with the change On 21 Oct 2017 10:58, Andy Dennis andy.dennis@btinternet.com wrote:We should cancel this one, however that is done. However, I don't think the code is quite right still. The master now does discover correctly but I don't think is correctly responding to the echo turn on and off requests,  which I had also fixed in my repository. I need to check the cause of that over the next couple of days, when I can fit it in. I think that may be the reason for the json format change I added in this PR On 21 Oct 2017 10:05, probonopd notifications@github.com wrote:So how should we proceed with this PR?

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

probonopd commented 7 years ago

Closing this PR as suggested by @Quanghoster. Please open a new one. Thanks.

Quanghoster commented 7 years ago

Once I have figured this part out I'll put some time in to spiffs On 21 Oct 2017 11:16, Andy Dennis andy.dennis@btinternet.com wrote:If I find the issue I will post an updated PR with the change On 21 Oct 2017 10:58, Andy Dennis andy.dennis@btinternet.com wrote:We should cancel this one, however that is done. However, I don't think the code is quite right still. The master now does discover correctly but I don't think is correctly responding to the echo turn on and off requests,  which I had also fixed in my repository. I need to check the cause of that over the next couple of days, when I can fit it in. I think that may be the reason for the json format change I added in this PR On 21 Oct 2017 10:05, probonopd notifications@github.com wrote:So how should we proceed with this PR?

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