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

Candle Color capabilities no longer properly recognized? #48

Open cakefornoreason opened 2 years ago

cakefornoreason commented 2 years ago

I'm technically not sure whether this issue is related to this repo or this one (https://github.com/ristomatti/node-red-contrib-node-lifx) but after upgrading lifx-lan-client to v2.0 in my Node Red setup, my Candle Color bulbs register their only "capability" as temperature, not color in Node Red.

I've run into this before and the fix was actually a modification or update to products.json, but as far as I can tell, both the included products.json and the updated one available from LIFX have all of the bulb capabilities properly assigned and though I spotted the issue in the context of Node Red, the issue wasn't present before lifx-lan-client was updated.

Is there something weird going on with the PIDs in this package perhaps that's causing a light with a known PID not to be properly connected with the appropriate capabilities listed in products.json?

Glad to help troubleshoot, test, and clarify if it helps!

ristomatti commented 2 years ago

Thanks for the report. I didn't realise this would be pulled for the Node-RED node as well. :man_facepalming:

Unfortunately I don't own the particular product but I'll update my NR setup to use 2.0 as well and see if I notice something. As a workaround you should be able to downgrade lifx-lan-client by running npm install --save-exact lifx-lan-client@1.1.0 under your .node-red folder.

cakefornoreason commented 2 years ago

I'll have to see about rolling it back. I will say that to put you at ease, I don't think this is actually being auto-pulled when installing the Node Red node--I did the update to this manually knowing that it could cause trouble, so I think you're managing this properly and I'm the one trying to break my own setup. 😉

I've just had a long-standing issue with LIFX light nodes in Node Red occasionally emitting updates that actually belong to the wrong lights. I mitigated it in my own code so that it would ignore any updates that didn't come from the intended output node, but wondered if there was any chance this library update might fix it. I still wonder if it's related more to my eero network, as I haven't seen any similar reports from anyone else.

As one other lead, the only other time I've seen something like this was when products.json was very out of date and thus had no knowledge of the newer light I was using. I was always able to fix it by creating an item in products.json to fill in for the missing one. That's why I think that something must have changed in the PID identification process, because products.json seems to be complete and the bulb worked before.

At any rate, sorry to scare you, but happy to test anything you like!

ristomatti commented 2 years ago

Ah true, the version is set to ^1.0.1, so should not upgrade to a major version but should to the latest 1.x.x. It seems I had still been sloppy and forgot to push tags also to this repo. Some other tags were missing also, all pushed now.

Tags allow easier comparison. I can't spot anything that could explain the issue you're having with Candle Color bulbs. Can you? https://github.com/node-lifx/lifx-lan-client/compare/v1.1.0...v2.0.0

The previous release contained a lot more changes (including changes to products.json): https://github.com/node-lifx/lifx-lan-client/compare/v1.0.5...v1.1.0

Diff for products.json from that release only: https://github.com/node-lifx/lifx-lan-client/commit/e9205b717cf5ad42591014c7329106b771fe545d. Now that you've mentioned, it seems on the latest update to products.json either LIFX or myself (by mistake) have removed PID 56 completely which used to be:

        "pid": 56,
        "name": "LIFX Beam",
        "features": {
          "color": true,
          "infrared": false,
          "multizone": true,
          "temperature_range": [
            2500,
            9000
          ],
          "chain": false
        }

Was it the latest products file version released by LIFX that you diffed the file against?

ristomatti commented 2 years ago

Oh, there seems to be various updates to the file that we're missing. Including this commit: https://github.com/LIFX/products/commit/dd692a341125a1c7b7397e058ee0d0f20f120a36. If you want to help, could you try replacing the file under node_modules/lifx-lan-client/src/lifx with the latest version, then restart NR and see if this fixes your issue?

ristomatti commented 2 years ago

Actually, I've created a PR (https://github.com/node-lifx/lifx-lan-client/pull/49) with the new release so that I don't forget to try it out myself. You can also try installing the PR's version directly from GitHub with npm install node-lifx/lifx-lan-client#products-file-update. If you do, please let me know if it fixes the issue or if it works at all. :slightly_smiling_face:

ristomatti commented 2 years ago

I've just had a long-standing issue with LIFX light nodes in Node Red occasionally emitting updates that actually belong to the wrong lights. I mitigated it in my own code so that it would ignore any updates that didn't come from the intended output node, but wondered if there was any chance this library update might fix it. I still wonder if it's related more to my eero network, as I haven't seen any similar reports from anyone else.

I haven't noticed or heard about this issue myself either. I've been using this library for at least 5 years both by itself and in Node-RED via node-red-contrib-node-lifx. This is when including the time using them before forking.

But if the issue is not related to your setup some way or another, I'd suspect the NR plugin to be the culprit rather than lifx-lan-client. The plugin's codebase (which I've "inherited") has a lot of "code smells" that make me cringe every time I end up looking at it. Despite this, it has worked good enough for me. There's a lot of improvements to it that have occurred to me over the years though. Enough that I'd rather create a new one from scratch than try to work with the existing code and risk regressions to people using it. I just have too many ongoing projects besides work to go ahead and do that. All my lights are still LIFX though so I still have a nagging voice in my head telling me to do it someday. :slightly_smiling_face:

cakefornoreason commented 2 years ago

Okay, so I've worked with this a bit and have some interesting observations, but no clear answers.

--No matter what manipulation I did to the lifx-lan-client as it was installed, I could not get this to work correctly with the existing install. --The only way I was able to get it working was to fully remove node-red-contrib-node-lifx AND lifx-lan-client from my Node Red installation and reinstall only node-red-contrib-node-lifx, allowing it to install lifx-lan-client@1.1.0 as a dependency. --In that version, however, products.json does contain an error in PID 68 saying color is false when it should be true. I fixed that by hand and it works fine after the change. --I also attempted to take that working fresh-install state of node-red-contrib-node-lifx and modify only products.json in lifx-lan-client to use the newest products.json and that works fine too.

What all this adds up to for me is that it seems something strange happened when I tried to upgrade lifx-lan-clientseparately from node-red-contrib-node-lifx, so I actually think it might be safe to try (and I'd be glad to test) an updated version of node-red-contrib-node-lifx that uses lifx-lan-client@2.0 as a dependency. It may also be worth warning people off upgrading lifx-lan-client on its own, but it could also just be my setup.

As for the extraneous status outputs, I'll have to watch for awhile longer to see if the recur, as now I'm wondering if that wasn't a side effect of my strange upgrade approach. That said, even if the code you inherited is a bit messy, you're right that it works more than it doesn't and I've been able to make it work for my purposes too, so I greatly appreciate that you're still maintaining it in whatever way you can! I just also have to imagine that trying to fix someone else's code is incredibly hard, especially when you have people like me who couldn't have built it in the first place bugging you about the strange ways it breaks!

At any rate, thanks for everything and if you would like me to test a new package that combines node-red-contrib-node-lifx and lifx-lan-client@2.0 please let me know!