ZeroOne3010 / yetanotherhueapi

A Java library for controlling Philips Hue lights. Available from the Maven Central.
MIT License
69 stars 20 forks source link

Why so restrictive? #45

Closed cyberpwnn closed 1 year ago

cyberpwnn commented 2 years ago

There's a lot of use cases for things that for some reason are just restricted.

Private constructors everywhere, Implementations hidden with protections, backing extremely basic interfaces (essentially hiding 90% of the information without reflection). Overly-simplified builders that prevent state-cloning for example. It feels like chewing rusty nails.

ZeroOne3010 commented 2 years ago

Hi Dan, thanks for your message! Sorry that you feel the library has let you down. 😞 As to your first point about light ids, I wonder what's your use case, i.e. why do you need the ids? The ids can be made available if there's a reason for it. Until now apparently nobody else has needed them yet. 🤷‍♂️

Regarding reading the raw JSON, have you checked out the getRaw() method of the main Hue class? That basically returns the raw JSON with minimal mapping.

Also, if you need to create a state with hue, brightness and saturation, you should use the State.builder().hue(1).saturation(2).brightness(3).on() pattern -- although now that you mention it, the hue is probably not needed/used if it's a non-color lamp, in which case I see your problem and agree that there is an issue!

I'm not exactly sure what you mean by state-cloning here? If you need to set the same state to multiple lights, just use the same State object for all of them. The class is immutable, so it's safe to use multiple times.

Note that I'm developing the library based on my own needs and user feedback only, so thanks again for your message! ❤️ 👍

cyberpwnn commented 2 years ago

I am trying to use multiple bridges to control a large area with over 70 lights in a super-state system. Basically All of the lights' colors are determined by nearby motion & noise, not what the user sets it to. So I need to know the position of the light spatially within the room.

When managing so many lights & bridges, it's essential to store / cache some state information about each light, such as where it is. Having the light id would be useful for such a task.

Now as for the state management, Im comparing the current light state (grabbed on first start) with the desired state, and update the light only when the state has changed. There are multi-level groups which all have their own "overlay states" I have some reflection tools & other apis I use that makes it far easier to just literally use a class (getters / setters / fields) than to use builders, hence the temptation to just use the damn class instead of using builders, that & when states change fast, builders / immutable classes tend to produce more garbage.

I'm pretty sure my use case is a bit further than ... Using this on android... So I can understand why some of these things are the way they are, I guess my opinion is to allow some freedom, some flexibility, because an API can never cover all use cases.

My current solution was to just to use ASM to un-final classes and publicize constructors & create methods in interfaces needed, so changes on your part are not needed, just took a lot more work than wanted.

fk0ff92 commented 2 years ago

Hey, I also like to use your API a lot. In my current project I'm trying to implement a light notification system, which also integrates other light systems. Currently I am generating an id using a hash method. Opening the id getter would also be very helpful and useful for me, so you have the chance to map the lamps uniquely in your own database. Before my hash method I built a modified version of yours that makes the id getter public. Maybe it would be nevertheless useful and nice if you can implement that. Greets from Germany and furthermore thank you very much for your work.

ZeroOne3010 commented 2 years ago

Hi @fk0ff92, thanks for chiming in! It seems that the ids are really required and it has been a mistake not to make them public in the past already.

I have now published version 2.4.0 where the getId() method has been made public for lights and rooms. That's a start at least, huh, @cyberpwnn? 😃 There's a couple of other additions as well, detailed in the CHANGELOG.md file.

mathias-ewald commented 2 years ago

I am using your library to automate a few things in my home. I have a config file that is used to define which lights to control and it would be nice to just store a lightId rather than a roomName and lightName. At the moment if I wanted to store only a lightId I would need to

  1. List all rooms
  2. For each room, iterate through all lights and match the ID
  3. Add all lights not associated to a room to the list

That's a lot of API calls for a simple "get entity by ID" and since it looks to me that the Hue API is structures like /api/USER/lights/ID, I too was wondering why you did it the way you did.

ZeroOne3010 commented 2 years ago

@mathias-ewald Hi there, thanks for your message! I suppose it all boils down to "can't get it right for everyone the first time"... If it's of any comfort, I've got a new version coming up, using the new API v2 that the Bridge provides, and that one will have a simple and clear method public Map<UUID, Light> getLights() which you can use to query all the lights at once and to get that one single light too.

ZeroOne3010 commented 1 year ago

Hi all! It took "a bit" longer than anticipated, but 3.0.0-alpha is now out! :tada: See if the library feels more ergonomic now. I'm just going to go ahead and close this issue, but feel free to either still comment here or to open new issue(s) to let me know what you think about it now! Any and all feedback would be much appreciated so that I could eventually promote the version to 3.0.0. Thanks for your patience! :pray: