Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can't get API to change the colour #4

Open
jamesmgreen25 opened this issue Jan 3, 2017 · 17 comments
Open

Can't get API to change the colour #4

jamesmgreen25 opened this issue Jan 3, 2017 · 17 comments

Comments

@jamesmgreen25
Copy link

Hi sidoh,

Thanks for your hard work - I was halfway through reverse engineering it myself before I found yours!

I'm trying to get this API to work with my LED device. I can get it to turn on and off, and to give full status readouts, but for the life of me I can't get it to update the colour of the LEDs. I've tried from the command line and from inside a script.

Is there anything you can suggest that I might try? I'd be very grateful for any insight you might have.

Many thanks,

James

@sidoh
Copy link
Owner

sidoh commented Jan 4, 2017

Which device are you using? I'm using this thing. Could be that the TCP APIs differ slightly.

@jamesmgreen25
Copy link
Author

jamesmgreen25 commented Jan 4, 2017 via email

@sidoh
Copy link
Owner

sidoh commented Jan 4, 2017

Ah, interesting. Let me know what you find! Would love to incorporate more devices if it's not too hard.

Packet structure for this project is here: https://github.com/sidoh/ledenet_api/tree/master/lib/ledenet/packets

@jamesmgreen25
Copy link
Author

I've had another look at my findings from before, and compared them more closely to the packets your code sends. I now have a list of packets that work when sent manually (at least for colour changing). I've noticed three things:

  1. The packets I caught sending from my phone had 9 bytes of data. There was an extra pair of 0s. I assume this is because my controller runs RGB, WW and CW too (though I only use RGB at the moment).

  2. The "remote or local" thing really doesn't seem to make any difference (as you mention in a comment in the code). The packets I'd traced from my phone simply sent 0x31, the RGB values, then 4 pairs of 00s and the checksum, and these work fine.

  3. The final byte for these 9 byte packets is calculated by getting the checksum from the first 8 (I used this one) and subtracting it from 100 (e.g. Green - 31 00 ff 00 00 00 00 00 has d0, 100-d0 = 30, so 31 00 ff 00 00 00 00 00 30 is the working packet).

At this stage I have a list of packets that, when sent, operate the controller as desired. I don't know how difficult it might be to include all this in the project, but I shall have a go tomorrow and see whether I can get it working (it's about 3am now and I'm tired). Presumably, if I'm right about the extra byte being for the CW channel, it would be possible in theory to update the project to be able to drive that too.

@sidoh
Copy link
Owner

sidoh commented Jan 4, 2017

Ah, yeah. This definitely makes sense. If this ends up being the difference, probably wouldn't be too hard to incorporate it without muddling the interface.

Maybe works out to be the same thing, but I calculate the checksum using sum(packet bytes) % 0x100: https://github.com/sidoh/ledenet_api/blob/master/lib/ledenet/packets/fields/checksum.rb

@jamesmgreen25
Copy link
Author

The checksum does in fact work the same way - I'd not clocked that before.

I've forked the repo and I'm having a go at modifying it. I've identified the following regarding the status response packets, adapting your comments in status_response.rb for my findings:

      On FID   SP RR GG BB WW ?? CW ?? CS
81:25:23:61:21:00:00:ff:6a:26:01:ff:0f:e9
81:25:23:61:21:00:00:ff:6a:26:01:52:0f:3c
81:25:23:25:21:0f:ff:00:00:26:01:52:5a:f0
81:25:23:25:21:04:ff:a6:00:26:01:52:5a:8b
81:25:23:25:21:04:ff:aa:aa:26:01:00:0f:9c

Somewhat strange that the cool white is stored in the 12th not the 11th - perhaps there's another feature of the device used by that byte.

I've managed to adapt it now so that (using the command line app), the --status flag reveals cool-white data and so that the --c flag sets it. I would assume that this change will break functionality on RGBW devices (due to the extra byte) but I don't have one to test. What do you suppose the best course of action is from here? Extra logic on API initialization to include/exclude the CW bytes?

@sidoh
Copy link
Owner

sidoh commented Jan 4, 2017

Yeah, I think that makes sense. Guess there are two high-level approaches I'd consider:

  1. Add another parameter to API initialization here
  2. Pull out common parts of API, stuff what's different into separate implementations of LEDENET::Api and have it delegate to the common methods where there's overlap.

(1) is probably easier, but involves runtime branching, which I'd prefer to avoid. (2) seems nicer in theory, but might end up being a more of a mess than it's worth.

Would also need to update the packet definitions.

It sounds like you're saying that the CW byte is the final byte of the packet -- is that right? I think that complicates the way the checksum byte is being calculated. :(

@sidoh
Copy link
Owner

sidoh commented Jan 4, 2017

I guess (2) is probably also nicer if there are other devices that have slightly different packet structures.

@jamesmgreen25
Copy link
Author

No, sorry - the CW byte is 6th, so the packet 31:00:00:00:00:ff:00:0f:3f sets RGB and WW to 0, and sets CW to 255/ff). Incidentally, I got that packet by Wiresharking the modified command line app. Fortunately, the checksum calculation is the same - I didn't need to touch that, only the checksum is now the 9th byte not the 8th. Here are the changes I've made so far.

I think you're right that 1. is probably easier to implement, but 2. may prove better in the long run. I think I'm understanding that the plan would be to have a common class and then inheriting classes that are then used for each kind of device? Would it be simpler to just create another class which inherits the existing LEDENET::Api and overrides where necessary? Forgive me if I'm being a bit dense here, or there's something I'm missing.

@sidoh
Copy link
Owner

sidoh commented Jan 4, 2017

Got it, makes sense.

To make sure I'm following -- looks like the structure of the status packet is unchanged (with the 24 byte unused payload changing to 8 unused, 8 CW, 8 unused), but the change color packet adds an extra byte. Is that right?

I was thinking composition might be cleaner here, but it could get too messy to have delegate methods for everything that's common.

Definitely not being dense. Thanks for digging into this!

@jamesmgreen25
Copy link
Author

Yes, the status packet is the same - just 8 bytes of the 24 "unused payload" is used for the CW value. I've adjusted the status response only to separate out the used 8 bytes from this section of the packet. The colour change packet does indeed carry an extra byte.

The 'functions' code seems to work for my device, but I don't tend to use them so have focused on just the colours for now.

It seems that there really isn't much difference between the two - it looks like a question of just adjusting the packet depending on the device I suppose. It suddenly occurs to me that I haven't looked at device discovery, and whether that might signal the type of device.

OK, great - I'm happy to defer to your opinion on how to proceed now. From my perspective, I've now got it working as I want on my machine, but I'd be more than glad to help if you're still keen to adapt your project to include this, and I can be of use in doing so.

@sidoh
Copy link
Owner

sidoh commented Jan 4, 2017

Yeah, would love to adapt this code to support the device you have. It's actually significantly cheaper than the ones I developed against. Would imagine plenty of people would prefer to use a controller that's half the price. I just ordered one.

The device discovery response packet doesn't have a ton of info. The "model number" mine responds with looks to be (or at least contain) a part number for a WiFi module. I think the discovery aspect is implemented by this module (some info in the code comments). Not sure if your device uses the same module. Maybe it's using ESP8266 or something.

@sidoh
Copy link
Owner

sidoh commented Jan 4, 2017

Super happy to collaborate on updates to this project. If you wanna throw up a PR, I'll definitely take a look. I'll try to get around to it myself in coming weeks if you'd prefer that.

@jamesmgreen25
Copy link
Author

Oh right - yeah, makes sense. Probably why I bought it at the time (had the thing nearly six months now).

ledenet-ufo --list gives the model number as HF-LPB100-ZJ200 and the MAC address begins ac:cf:23.

Fantastic. I'll have a bit of a go and see if I can manage an elegant way to include both options - no harm in trying - and then I'll submit the PR for review and thoughts.

@markusressel
Copy link

Hey there,
I have written a python api based off of this library that (from what i've read here) seems to implement this exact protocol. I don't want to advertise my project too much since all of the hard work was already done by @sidoh and I gladly used it, but for anyone interested:
https://github.com/markusressel/sunix-ledstrip-controller-client

Huge thx again to @sidoh

@sidoh
Copy link
Owner

sidoh commented Oct 2, 2017

Nice, @markusressel! This one might be of interest as well. Not sure how closely it relates to the hardware you're dealing with.

@peterevertz
Copy link

I have mad a fork for the 5 channel Controller:
https://github.com/peterevertz/ledenet_api

Thanks to sidoh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants