4JX / L5P-Keyboard-RGB

Cross platform software to control the RGB/lighting of the 4 zone keyboard included in the 2020, 2021, 2022 and 2023 lineup of the Lenovo Legion laptops. Works on Windows and Linux.
GNU General Public License v3.0
308 stars 39 forks source link

Console window appearing everytime the app gets opened #89

Closed Rodrxx closed 11 months ago

Rodrxx commented 1 year ago

Since last version v18 everytime the pc boots up and opens the RGB app, now there is a console window that also opens with it, i dont know if was the updated code of the app that causes this, but in the console window i cant type anything, is just a giant black box that doesnt do anything and i can close it perfectly, tried finding solutions from my end if it was something that windows updated and being the reason why the console window is appearing, but nothing so i decided to post this "issue" here.

4JX commented 1 year ago

That's weird, I cannot reproduce it on my side.

For overcomplication reasons the same executable shares the code to run both as a command line program and a GUI. If it detects it is not running from a CLI (aka GUI mode) it is supposed to clear the command line that pops up on Windows (some platform specific behaviour I don't know why it happens) and move on with execution.

Can you try legion-kb-rgb-cli-bug.zip and see if it changes anything? It should delay the clearing of the CLI till a bit later in execution, which sadly has the side effect of flashing the cli window briefly.

Also, what OS are you using?

Rodrxx commented 1 year ago

image That version you showed me pop-ups this.

Meanwhile the main version i downloaded shows me this: image

Im using Windows 11 22H2, but only the newer version of L5P RGB shows me the black command window

ccelik97 commented 1 year ago

I too have the same issue on Windows 11 Version 22H2 Insider Beta (Build 22623.885), on Legion 5 2020 (15ARH05H).

For the time being I wrote the following AutoHotKey script to launch the main .exe file from it: Usage: Compile it, and place it in the same directory as the legion-kb-rgb.exe file & run it.

Process, Close, legion-kb-rgb.exe

Run, legion-kb-rgb.exe,, Hide
Sleep, 500
WinShow, Legion Keyboard RGB Control

WinWait, Legion Keyboard RGB Control
WinClose

Return

It's not perfect as the GUI is being displayed briefly on startup but other than that it works fine here.

4JX commented 1 year ago

Can you try this version and tell me what it outputs/does on startup? legion-kb-rgb-cli-bug2.zip

ccelik97 commented 1 year ago

legion-kb-rgb-cli-bug2.zip

I extracted the .exe file to my Desktop, double clicked to run it and 2 windows appeared: 1) An empty CMD window, waiting. 2) A window without a title that says: "Current console count is: 1" (without quotes) with a Close button. When closed another such window appears and says: "Not running from a console, attempting to clear windows". When that closed too another such window opens and says: "Free console result is: 1". And only when that 3rd window is closed the main app UI appears.

I'll add it to my system startup and tell that too.

Update: The same exact things happen on system startup too.

4JX commented 1 year ago

Does the console window not get cleared/removed after clicking ok/closing the "attempting to clear windows" prompt?

ccelik97 commented 1 year ago

Nope, the console window stays open.

Edit: I think I understand what you're trying to do here with this build. Here on Windows 11 I have Windows Terminal (it's the default on 11 since last month) set as my default terminal app and it (a CMD tab) stays open with this build.

But if I set the Console Host as the default terminal then the window gets closed after the "attempting to clear windows".

What Windows version are you using there, 10?

4JX commented 1 year ago

What Windows version are you using there, 10?

Yes, the program calls an OS level function to clear the console windows. I'll have to look into why the Windows Terminal does not respect it (and possibly upgrade my OS).

ccelik97 commented 1 year ago

(and possibly upgrade my OS)

I see then, take your time.

Rodrxx commented 1 year ago

Windows 11 here, my results are exactly the same as @ccelik97 mentioned above so maybe there isnt a difference by OS.

Rodrxx commented 1 year ago

Got news here, it was kindly mitigated since it right clicked the shortcut in shell:startup folder and then select it to start minimized, now no console window is appearing and the GUI just chill down there but still i have to close it

ccelik97 commented 1 year ago

Got news here, it was kindly mitigated since it right clicked the shortcut in shell:startup folder and then select it to start minimized, now no console window is appearing and the GUI just chill down there but still i have to close it

Right, because "Start Minimized" setting doesn't respect the environment's default terminal setting yet (but I read that it too will soon) so it launches ConHost which is fine with this app.

So the number of possible workarounds we have here has increased to 2 but having the app to play nicely with new system defaults would be definitely better.

EntityXXX commented 1 year ago

I had the same problem, but it would run just fine if I went into the command prompt (not the one it launches, the regular one) and put in the file path and hit enter.

4JX commented 11 months ago

I have finally reached enlightenment found a way about making this probably work. https://github.com/4JX/L5P-Keyboard-RGB/actions/runs/6457619451

Please backup your current config.json or add a saturation_boost entry into any saved profiles that have the AmbientLight effect (ie. "effect": {"AmbientLight": {"fps": 0, "saturation_boost": 0.0}},) before testing as the program currently just overwrites files it cannot load for the current version (working on improving that one).

Does this one work on your systems? It should

ccelik97 commented 11 months ago

https://github.com/4JX/L5P-Keyboard-RGB/actions/runs/6457619451

Does this one work on your systems? It should

  • Not display a console window by default if ran by double-clicking or similar
  • Give CLI output if ran from a command prompt
  • Display a command line if you press the "scroll" (logs) button on the top right.

I tried it and it all seems to be working as you mentioned/as expected. Good job.

But here's a thing: The AmbientLight effect reacts with a delay, no matter what the FPS setting is set to. Contrary to that, the older builds including the one I made a PR from (#92) are all reacting instantly. So, unless it's something you're aware of it may be an area to take a look into. (The other reactive effect, Ripple, is fine though: No such unexpected delays.)

System details:

4JX commented 11 months ago

The AmbientLight effect reacts with a delay, no matter what the FPS setting is set to. Contrary to that, the older builds ... are all reacting instantly.

Do you mean the older releases from the release tab? Those were compiled in "release" mode (since your PR was targeted at the main branch it also triggered the release workflow along with the debug builds one) which takes longer to compile.

The build I linked is a "debug" build which is expected to be generally slower. You can try to compile the 19.3 branch yourself to see if that improves things.

(A release build will also be made when the final version is uploaded to the releases tab)

ccelik97 commented 11 months ago

I see. If it's a known thing and that's why (a non-issue) then I don't have a problem with it. I'm happy that the console thing is finally resolved now.

Btw since it's been a while since that PR I made and there have been quite a lot of changes to the project, should I wait until there's a new release build made or before to redo the AmbientLightWarmerDesaturated effect (& send a new PR)?

Or I mean since there's a saturation control I'd make AmbientLightWarmer only?

Or should I look into implementing a color temperature slider instead similarly to the saturation slider and leave the effects alone?

4JX commented 11 months ago

Or should I look into implementing a color temperature slider instead similarly to the saturation slider and leave the effects alone?

Yes that would be ideal, for further discussion/doubts please use the discussions tab

Issue is fixed in 0.19.3