node-lifx / lifx-lan-client

Canonical fork of the number one Node.js 💡 LIFX LAN protocol implementation.
MIT License
43 stars 20 forks source link

Compatibility Question #20

Closed Shakesbeard closed 4 years ago

Shakesbeard commented 4 years ago

Hi guys, I came here by reading up on the node-lifx repo conversation. There you mentioned you wanted to maintain backward compatibility to the version Marius created. Am still using that 0.8.0 version. I wondered whether your current build can just be used as plug and play replacement or what culprits are to to await there. Further using the lib from Marius I see a weird saw like memory leakage issue. See here: image It seems to be related to polling using the getState method. Did anyone ever find something out about that?

Sawtaytoes commented 4 years ago

lifx-lan-client does maintain backwards-compatibility with node-lifx, but the underlying code has quite a few improvements.

I run lifx-lan-client on a Raspberry Pi at home and have noticed no memory leaks. When I used node-lifx, I also did not notice any memory leaks.

This may be because of how I've configured my application, or it could be something on your end. If your code is public, that would help us take a look and figure out what's going on.

Shakesbeard commented 4 years ago

Thanks. Good to know. Well unfortunately I don't have the codebase public. Am working on a Homey Driver App for LIFX LAN support because Athom is phasing out the old SDK. So I had to port the driver to the new SDK myself as the official LIFX app Athom provides is using the slowpoke Cloud API only.

I will give your library here a try as soon as I get some spare time and see how it works for me.

Shakesbeard commented 4 years ago

So yeah, it literally was a plug and play replacement :D Awesome! I can see this slightly strange RAM effect still though: image With your library the interval of the big teeth became shorter by roughly an hour. I cannot see anything explaining this pattern in the driver app, The only thing being done all the time is a setInterval of 10s which calls "getState(function (err, data) {..." per LIFX device registered by the app. Unfortunately am also not really a nodejs dev. Am .NET object pusher ^^''. But everything else in the app is done event based, meaning if Homey triggers a flow card to send a command. So in the graph above everything happening from on about 6am is this polling interval. I suspect console.log to be a possible culprit but the Athom devs told me this should not eat RAM. So.. well.. if you got any hints what I could possibly check for, that would be very welcome.

ristomatti commented 4 years ago

@Shakesbeard I have not noticed any memory leakage either. I'm also using lifx-lan-client (and previously node-lifx) indirectly as part of my home automation system. The library is a dependency of node-red-contrib-node-lifx and I would assume it would be calling getState all the time also.

This is just a wild guess but could it be that one or some of the lights isn't responding to getState and you're just requesting the status unsuccesfully on every 10 sec cycle? This could cause the pending getState requests to "pile up" hiking up the memory usage. Eventually the requests would time out and the garbage collector would free up the used memory creating the sawtooth pattern.

Shakesbeard commented 4 years ago

@ristomatti Hmm.. I debug mode I seen a few times when no data was returned (should prolly evaluate the error value maybe) I cannot see any issues on the Homey userinterface. It looks like it polls state correctly for all configured bulbs.

Well as long as the app does not exceed 30MB am fine. I think they have a hard limit at 35MB for driver apps, where the app gets killed off for misbehaving.

Thank you for that hint. I will try to instrument my code better and do some more debugging towards that possible condition.

One more silly question though. When the callback resturns with data == null and possibly an error value set, did the getState call get freed up then already or is it still doing something in the back? (just to understand your statement a bit better)

Shakesbeard commented 4 years ago

Alright. I eventually found the culprit and it has nothing to do with the lifx lib itself it seems. The GetState calls work fine. I do have a few error situations but I don't think those will be a major issue at all. Thank you guys for the insight and help.

Sawtaytoes commented 4 years ago

@Shakesbeard I know you closed this issue, but I want to add in some of my findings: image

This is the log output from my LIFX controller software. I have a listener not just on new lights, but on offline and online: image

This is because sometimes lights might disappear and then re-assert they're back. Sometimes lights do get turned off and not turned back on. In those cases, it's good to know they're offline for good and not call getState on them.

I use these events together to ensure I always have the latest light object references and ensure my list of online lights is up-to-date.

Shakesbeard commented 4 years ago

@Sawtaytoes Thanks for the note but I had that covered already. The polling is disabled and reenabled in such cases. That works smoothly.

ristomatti commented 4 years ago

@Sawtaytoes For home automation I would actually suggest ignoring the light status unless everything works flawlessy;. I've found out the lights are often responding to the app even when they show offline in Node-red. What I do is run the discovery once at startup and then just send the packages.

Shakesbeard commented 4 years ago

Hmm. I want to reopen this after some observations. Could it be that the "discovery" causes a bit of RAM nomming over time? After my last investigation I found a culprit ofr exzessive RAM eat up and removed that. But I can still see a continous RAM usage increase over time. Even though the app actually does nothing. So I suspect it might be the discovery process running in the background. However, I would like to keep that on for the lights gone and coming back events because I expose them as trigger events to the Homey eco system for few reasons. Has any of you running discovery continously and can see if you see something similiar?

Shakesbeard commented 4 years ago

Well. There is only a very minimal things. I figured I can manually trigger the GC whilst my app is idle which will reduce garbadge a lot.

ristomatti commented 4 years ago

@Shakesbeard It might very well be that there's a memory leak in the library. You're welcome to open a new issue on that. Especially if you can provide minimal test code that demonstrates the issue (to exclude possible issues with your other code).