search

originallyus / node-red-contrib-alexa-local

An easy-to-use NodeRED node for adding Alexa capability to NodeRED. NO Alexa Skills required.
107 stars 25 forks source link

Alexa will not discover node's new name after you rename it #12

Closed PKGeorgiev closed 6 years ago

PKGeorgiev commented 6 years ago

Recently I encountered a small issue with Alexa-local node. When you rename a node its new name is not discovered by Alexa.

Here are the steps to reproduce it:

1) Create a flow in NodeRED and put Alexa node. Name it "device 1" (the name does not matter) 2) Ask Alexa to discover devices. It will find "device 1" (you can check in the website) 3) Rename the node to "device 2" & Deploy the flow in NodeRED 4) Ask Alexa to discover devices and wait. In Alexa > Devices there still be "device 1" (i.e. the name was not updated to "device 2") 7) Restart NodeRED 8) Ask Alexa to discover devices and wait. This time device's name is updated to "device 2"

Here is a video to demonstrate the issue: https://monosnap.com/file/MvFHFKfUMskckdwp33BxipnTMeC5mH

It seems that handleHueApiRequestFunction() is called multiple times. May be http server handler was not cleaned up after re-deploy in NodeRED.

PKGeorgiev commented 6 years ago

I found that when you re-deply, the allLightsConfig variable holds xml with the previous device name. So if you run Alexa rediscovery - the node returns stale data.

Interestingly when you rename & deploy the node is initialized with the new devicename value.

torinnguyen commented 6 years ago

allLightsConfig variable is a local variable only within that function scope, it will get destroyed as soon as that function finish (which is almost immediately). After renaming a device, you will need to ask Alexa to discover devices immediately for that to apply. You missed that step between step 4 & 5.

Here's a video of the current/latest version https://monosnap.com/file/fUwcFsGFG1yHFBAX8WpP77vuyk9YEj (please download the video if it does not stream directly online)

PKGeorgiev commented 6 years ago

May be I couldn't explain it well. What I mean is that handleHueApiRequestFunction() receives old devicename value so when you rename a node and activate discovery Alexa does not recognize the new name. You have to restart NodeRED for this to happen. And this is definitely a bug. I suspect that onclose does not properly cleanup http handlers or something similar. And this is something that may have to be investigated. Have you tried steps 4 and 6 (step 5 is not interesting)?

torinnguyen commented 6 years ago

I'd like to see a video of the steps you actually did. I don't understand your description

PKGeorgiev commented 6 years ago

Ok, I'll refine my question and will prepare a video.

PKGeorgiev commented 6 years ago

I've updated the description and steps. You can also see the video.

torinnguyen commented 6 years ago

I've replicated your exactly flow but it does not happen on my system. However, I have a theory. Most likely it's because you're running NodeRed on Windows machine, the httpServer does not close completely due to keep-alive connections. There are actually 2 servers running after you perform a new deployment. If you're interested in the details, the links are at the bottom.

I've publish a new version with 'stoppable' module to force close any httpServer outstanding. You can check this commit for the code #2642

I am not able to replicate the issue, so it's up to you to test it out and let me know if it works.

http server close() doesn't end http keep-alive connections https://github.com/nodejs/node-v0.x-archive/issues/9066 https://github.com/nodejs/node/issues/2642#issuecomment-302831167