search

chestm007 / linux_thermaltake_riing

Python driver and daemon to control thermaltake Riing fans and pumps
GNU General Public License v2.0
61 stars 25 forks source link

Public API wanted #29

Open henryruhs opened 5 years ago

henryruhs commented 5 years ago

Is there an public API of shorthand methods available? I like to integrate this libary into Chroma Feedback. https://github.com/redaxmedia/chroma-feedback

Something like:

termaltake = Termaltake()

termaltake.turn_light_on()
termaltake.set_color()
termaltake.set_fan_speed()

I think you get the idea. Let me know if you need an helpful hand but first we need an API to be defined.

chestm007 commented 5 years ago

This has been on my mind for a while now, i did initially implement a dbus api, but this software was changing far too rapidly to keep it in sync without massive overhead, i should hopefully have time this weekend to look through the code but yes - im 100% in favor of this!

henryruhs commented 5 years ago

This would be really great... I suggest you create one API for each manager of this library instead of mixing things up.

You could post an concept here before actually coding it.

chestm007 commented 5 years ago

Due to the live nature of the daemon process, at the minimum, to provide all current functionality over an API there would need to be some form of RPC/JSON api, im leaning slightly towards JSON over http as it would be the most flexible in terms of allowing any language to be used to interface with it.

I had planned to provide this mechanism so people can create GUI front ends for this daemon - life just got in the way.

The last time i played around with this idea, there were mechanisms in the daemon currently to "hotswap" in a new lighting or fan manager thread on the running daemon - so it shouldnt be overly difficult.

Either way it goes i plan to create a navie python api client library fairly similar to what you have outlined above

ttapi = ThermaltakeRGBApi.Client()
ttapi.connect()  # connect seperate to allow reconnection incase something goes bad
light_manager = ThermaltakeRGB.lighting.Full(r=100, g=100, b=100)  # could just as easily be a json object.
ttapi.set_lighting(light_manager)  # set lights to.... light grey?
# copy paste the above, refactored for fan managers
ttapi.save()  # this would be a "nice to have" - basically save the current running setup to the config.

basically this way we can add and remove lighting managers, and only need to declare a new type in the client (you could also get a list/argument spec directly from the daemon and dynamically build those request objects)

The difficult part would be "intermittent" effects, eg. "turn red for 10 seconds"... this sort of thing would possibly need to be heavily reliant on the client implementing the api to handle setting, and reverting the lighting effects in use, we could (and probably should) provide a method of getting the current running effect settings, either to be displayed in a gui when loaded, or to be used to "revert" the settings back.

henryruhs commented 5 years ago

I appreciate that you think forward to clients/frontends to remote control the Thermaltake devices but do we really need this in a near future and is there an usecase for this? It would be really great to have it for sure but "You Aren't Gonna Need It" and "Single Responsibility Principle" crossed my mind while reading your response.

General

Pro

Contra

henryruhs commented 5 years ago

I could describe the basic workflow of my application as this could be an inspiration for the native Python API:

  1. Create and API factory and throw Exceptions if something goes wrong
  2. Get all devices/groups and iterate over them
  3. Set colors for each device or group

Checkout a bunch of implementations: chroma_feedback/consumer

Your code:

ttapi = ThermaltakeRGBApi.Client()
ttapi.connect()
light_manager = ThermaltakeRGB.lighting.Full(r=100, g=100, b=100)
ttapi.set_lighting(light_manager)
ttapi.save()

Wanted code as I don't mind about the internal stuff:

ttapi = ThermaltakeRGBApi.Client()
ttapi.connect()

# it should be device/group related and not general

devices = ttapi.get_devices()
for device in devices:
    device.light_manager.set_rgb(r=100, g=100, b=100)

As you see, we might spend more time into defining the complete API than start coding! :-)

chestm007 commented 5 years ago 
devices = ttapi.get_devices()
for device in devices:
    device.light_manager.set_rgb(r=100, g=100, b=100)

this specific functionality isn't supported as yet, a lighting effect is set across all devices - It's on my to do list, I just haven't required it so far so its pretty far down it...

an detour via RPC/JSON is annoying

i think this assumption is wrong, either that or I'm misinterpreting something.

The software currently runs as a daemon process, this allows it to react to system events (cpu temperature namely) and continually send commands to the thermaltake usb controllers (fan speed, lighting(temperature depended lighting especially)).

currently what i understand as your request/requirement is to expose certain parts of this code to be used directly as a library, without the daemon running - That would work fine on the surface, but would massively conflict with any running daemon process trying to set lighting effects and fan speeds, and essentially mean you have to run the daemon or use the API, not both, which isn't ideal.

Due to the daemonic nature of this software, i believe the API would be far better implemented using either a python native RPC (RPyC, pyro, etc) or JSON api using a http or dbus interface.

I absolutely believe we'll spend more time designing the api then coding it, tis just the nature of API's intended to be externally used :)

chestm007 commented 5 years ago

Having said all of that, i have had the idea for a while that i should think about breaking this project up into library code (device interaction etc) and controller code (the actual daemon)

henryruhs commented 5 years ago

this specific functionality isn't supported as yet, a lighting effect is set across all devices

No problem, I could live with an general API too as long there are no internals to take care of

ttapi = ThermaltakeRGBApi.Client()
ttapi.connect()
ttapi.light_manager.set_rgb(r=100, g=100, b=100)

i think this assumption is wrong, either that or I'm misinterpreting something

Maybe, I thought you were talking about sending raw commands to the daemon, I do not have a problem running the daemon as we need it like you said.

i should think about breaking this project up into library code

Don't do that - this is going to make things more complicated in terms of maintenance, testing, versioning and during development. As long there is no need for it, this is an massive waste of time.

absolutely believe we'll spend more time designing the api then coding it

Looking forward it! :-)

chestm007 commented 5 years ago

this is going to make things more complicated in terms of maintenance, testing, versioning and during development. As long there is no need for it, this is an massive waste of time.

That was more or less the conclusion i've come too everytime i think of it :)

In terms of designing the API, first i need to get a very basic interface put together, from there adding extra functionality should be trivial.

I'm 100% in favor of keeping the api fairly high level, it really should just be (client side) a case of:

In terms of not having to deal with the internals:

ttapi.light_manager.set_rgb(r=100, g=100, b=100)

as low as the usb controller level, the thermaltake products do have a concept of lighting effects (spectrum, full, etc) so i think atleast that level needs to be exposed.

also the actual RGB values are set in the same command as setting the effect and lighting speed, so we should probably allow that functionality too from the API.

something like:

ttapi.light_manager.set_rgb(effect='full', r=100, g=100, b=100, speed='fast')

could work, but then you lose the ability to set each individual LED without the method signatures becoming far too generic (entirely not even supported at this stage, but definitely another thing i want to do - the main motivation behind this, is i want feature parity with the windows software)

alternatively:

ttapi.light_manager.set_lighting({'effect': 'full', config: {'r': '100', 'g': '50', 'b': '40'}})

I feel like that has the potential to become a bit "unwieldy" but could easily be abstracted in the API client - and was the reasoning behind the initial pseudo code i wrote which involves instantiating a lighting effect class (basically meant to be a thin wrapper around a python dict, though could just as easily be a dict)

Either way, to properly implement this API we need at the least:

I might get started on hacking together a POC that just changes lighting effect to begin with, should get a good feel for the flow in doing so.

henryruhs commented 5 years ago

As mentioned before, I worked with a bunch of other RGB related libraries. Most of them use one method for each effect such as:

ttapi.light_manager.static(r=100, g=100, b=100)
ttapi.light_manager.pulsate(r=100, g=100, b=100)
ttapi.light_manager.breath(r=100, g=100, b=100)
ttapi.light_manager.spectrum()
ttapi.light_manager.wave()
ttapi.light_manager.turn_on()
ttapi.light_manager.turn_off()

This results a much cleaner high level API as we prevent unused parameters and can start with a little set in the beginning. Another benefit is, that the API describes itself just by it's given names.

Of course there could be something more low level too but this is not high prio:

ttapi.light_manager.set_effect( ... pass an config for custom pattern ... )
chestm007 commented 5 years ago

agreed!

henryruhs commented 5 years ago

Hello,

I created an issue on my project Chroma Feedback to show you the anatomy of an possible implementation. This is based on having an DeviceManager with each devices being able to have color functions and own properties to be listed and filtered via the command line.

Having global functions would be less powerful, the alternative implementation is described too.

Hopefully you get some time working on this soon.

chestm007 commented 5 years ago

Hey,

Thanks :) - I had the engine in my car basically blow up 2 weeks ago so i don't really have time at the moment (building the replacement). I will look when i have time, but I cant concretely say when that will be :/