CoolerMasterTechnology / Pi-Tool

Overclocking and button-remapping utility for the Raspberry Pi 4, designed for Pi Case 40
GNU General Public License v3.0
196 stars 18 forks source link

Button presses don't work when the GUI isn't running #7

Open Omoeba opened 3 years ago

Omoeba commented 3 years ago

I have a headless pi and set up pi-tool with VNC. However, after a reboot from the command line, the daemon doesn't respond to button presses that are configured in the GUI. [WARN] Could not find corresponding button action appeared in the daemon logs when I pressed the button. Button presses work again if I VNC into the pi and start the GUI.

mntns commented 3 years ago

Thanks for opening an issue! Indeed, the daemon doesn't persist any button mappings across reboots at the moment. Since the Pi Tool was designed with desktop usage in mind, it's unfortunately a wontfix for now, but if you or anyone else wants to look into it: The ButtonListener in the daemon would need to write the mappings to a file upon sync: https://github.com/CoolerMasterTechnology/Pi-Tool/blob/8a2892be57646d31fd7b72d341f9ccca632f7c5b/daemon/src/mapping.rs#L82 and load them during initialization https://github.com/CoolerMasterTechnology/Pi-Tool/blob/8a2892be57646d31fd7b72d341f9ccca632f7c5b/daemon/src/mapping.rs#L64 With this, headless usage should be possible after having configured the button mappings via VNC.

C0D3-M4513R commented 3 years ago

Maybe it should be considered seperating the two completely. This would also resolve #2 .

flatline4400 commented 3 years ago

"Since the Pi Tool was designed with desktop usage in mind, it's unfortunately a wontfix for now"

What? Sorry, this is dumb. If you're physically AT the desktop, why would you use the button on the Pi to do anything? The most obvious use of the button is for when you don't have (headless) or are not near the keyboard mouse, and need to do something.

It should be done as C0D3-M4513R says. Daemon, runs at boot. Config file in standard human readable format. Front end/GUI for management/monitoring. The button should always function, or what is the point?

This whole Coolermaster project has been such a joke from the get go, I swear...

C0D3-M4513R commented 3 years ago

I had a quick look at the code. The core issue is, that this issue means a whole rewrite of the whole project (or a complex translation unit), because the code is unessesarily complex. The Code should be much simpler, but because the Code wasn't desiged for this purpose, it won't be that easy. Also the hardware part is executed really well. For the Software bit, Coolermaster can't forsee all our use cases, so please be a little more understanding. Also please note, that I am not related to Coolermaster in any other way, outside of being a customer.

flatline4400 commented 3 years ago

All I am understanding is that, somehow, the software guys delivered their half of the product on time. People are only now getting the actual hardware. So kudos for that.

But forseeing use cases is what developers are supposed to do. I can't see any use case for this software. Nor does it fit the description of the product we paid for. I'm sorry if it hurts feelings, but this is a business transaction. The product is not as described. It does not function as described.

We paid for this. It was kickstarted as a lie and now sold as a retail product. We deserve to be delivered what was promised.

themasch commented 3 years ago

The core issue is, that this issue means a whole rewrite of the whole project (or a complex translation unit), because the code is unessesarily complex. The Code should be much simpler, but because the Code wasn't desiged for this purpose, it won't be that easy.

I do not think that is the case. I think the code is actually pretty reasonable. One could just implement a config/state file thats read on start. This could be deserialized into a config struct which also allows modification through a interior mutability (arc/mutex/refcell...) which also writes a serialized form of the config back to the file. References of this config struct could then passed to the subsystems (e.g. the ButtonListener mentioned by @mntns above).

This may even be extended later to support something like inotify for live reload, but I thats would going to far for a first step.

Am I missing something?

Oh, and yeah, I totally agree with both of you: not hating on anyone or anything here, its great that this repo exists and we can contibute. Totally fine that not all use cases are solved yet.

nobodypb commented 3 years ago

While I'd also like to have the pi-tool for headless usage, I agree with @themasch. Simple persistence can get added to the deamon quite easy and one could also write a CLI tool that emits the neccessary websocket commands or even provide a fancy webinterface. You'd really only need to port the existing app from Electron to web and maybe better add authentication. In this regards I consider this project really flexible and well designed, although perhaps not developed to the last detail. What's missing then are custom button actions, but that's something that should be reimplemented for CLI mode nevertheless. Who needs to start a browser in CLI anyway?

mntns commented 3 years ago

Just to chime in here: For persistence and regarding the daemon code, I think what @themasch outlined is the way to go. In terms of CLI usage, I wouldn't expose the daemon state as a config file, since part of the reason why the Pi Tool was designed is to abstract config files away. Like @nobodypb wrote, a simple CLI tool could be written that sends Websocket commands to the daemon (the API is completely documented here) and I like the idea of exposing the underlying React app of the GUI as web interface for remote usage.

themasch commented 3 years ago

Yeah, thats what I was thinking, too. The daemon should be the only once reading and writing those files. Everything else should just communicate with the deamon to query the current state and or change it.

@mntns I was unable to understand if the current GUI would be able to deal with a deamon that already has button actions configured. I saw something about a persisted config in the react code. Are these things currently persisted by the GUI? If so this could cause conflicts if the persisted config in the GUI and the one in the deamon do not match, right?

mntns commented 3 years ago

@themasch: Good point with potential conflicts of persisted button mappings in the GUI. Indeed there would have to be a few changes to the GUI code if the daemon persists the mappings and there is a CLI.

Currently the mappings are stored via redux-persist, but that could be thrown out and the daemon could be made the 'single source of truth' for the button mappings. Then there would also have to be an API method like FetchMappings so that the GUI can get the stored mappings during launch and populate the Redux store with it. But as long as there is no CLI I believe it would be fine to add persistence in the daemon without this change, since the sync would still be unidirectional (frontend -> daemon) and there is no other means of modifying the daemon state.

neggles commented 3 years ago

FWIW, if all you want is for the shutdown button to work, this application is completely unnecessary. The button is on the standard/default shutdown pin, GPIO 3, so all you have to do is add this to your /boot/config.txt:

# Enable shutdown button on GPIO 3
dtoverlay=gpio-shutdown

If you'd prefer to set the defaults explicitly:

# Enable shutdown button on GPIO 3
dtoverlay=gpio-shutdown,gpio_pin=3,active_low=1,gpio_pull=up,debounce=100

As a bonus, pressing the button while shut down will start the pi back up again. I think that works either way, but.