bigtreetech / BIGTREETECH-TouchScreenFirmware

support TFT35 V1.0/V1.1/V1.2/V2.0/V3.0, TFT28, TFT24 V1.1, TFT43, TFT50, TFT70
GNU General Public License v3.0
1.3k stars 1.65k forks source link

[FR] Add option to be able to control case light #1194

Closed mak0t0san closed 3 years ago

mak0t0san commented 3 years ago

Is your feature request related to a problem? Please describe. Unable to control case light from touch screen. Have to go into Marlin mode to be able to change case light.

Describe the solution you'd like Add a button (that is only shown if caseLightsBrightness in the struct MACHINESETTINGS is set to 1) that will take the user to a Case light menu. The Case Light menu will have options to increase or decrease the brightness, or to turn it on or off.

Describe alternatives you've considered The current alternative is to go into Marlin mode.

Additional context Case light gcode information is here: https://marlinfw.org/docs/gcode/M355.html

Sending M355 with no parameters will get the current values. Here are some examples:

Send: M355
Recv: echo:Case light: OFF
Send: M355  
Recv: echo:Case light: 255
oldman4U commented 3 years ago

Whats wrong with using the existing LED light function?

mak0t0san commented 3 years ago

Whats wrong with using the existing LED light function?

The LED light function controls Neopixels RGB LEDs. This is different compared to case light.

For example, I have a strip of LEDs connected to an unused heater output and am able to control the brightness by sending M355 gcode commands. The TouchScreenFirmware doesn't currently support M355.

I am currently working on an implementation to test it out, but would like to go through an official path so that new icons that match the current theme can be created.

oldman4U commented 3 years ago

Got it, thx.

Unfortunately there is nothing like an official path. In fact, this repository is a bit like the wild west where tickets are opened but never closed and where developers are working on the same stuff without knowing from each other. This happened several times and people left frustrated. Now we try to make everything a bit more organised and one part of it is to collect all FR's into a single ticket, and also to show, if an engineer is working on it or not. I already added your ticket to the pinned ticket #1170 and marked it as "under development". It would be great if you could close this ticket, but the same time use it for further discussion. Thank you

For a fast start, I would check if one of the existing Icons can be used. Do you know where you would like to add the control buttons in the UI?

mak0t0san commented 3 years ago

Yeah. Planning on adding it to the settings screen, after Connection. The interface will be similar to fan control, there will be a minus button on the left (to make the light dimmer), a plus button on the right (to make the light brighter), and then on the bottom left a toggle button that will turn the light on and off. Maybe another button to change the increment/decrement value. The fan speed control is annoying as it only increments by 1% at a time, so you have to tap on the button many many times to change the speed. I would like to give the user the option to change the increment value by either 1% or 5% or 10% at a time.

oldman4U commented 3 years ago

You are aware that it is not a setting but a feature...

But everything else is even more clicks away. So have you thought about adding it as a second option to Fan? Like speed and flow.

Makoto Schoppert notifications@github.com schrieb am Do. 22. Okt. 2020 um 16:42:

Yeah. Planning on adding it to the settings screen, after Connection. The interface will be similar to fan control, there will be a minus button on the left (to make the light dimmer), a plus button on the right (to make the light brighter), and then on the bottom left a toggle button that will turn the light on and off. Maybe another button to change the increment/decrement value. The fan speed control is annoying as it only increments by 1% at a time, so you have to tap on the button many many times to change the speed. I would like to give the user the option to change the increment value by either 1% or 5% or 10% at a time.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/bigtreetech/BIGTREETECH-TouchScreenFirmware/issues/1194#issuecomment-714543203, or unsubscribe https://github.com/notifications/unsubscribe-auth/AM6XKZCKFCMMJH4T4PEXC63SMBAGRANCNFSM4S2MGK6Q .

oldman4U commented 3 years ago

And how to access it during print?

mak0t0san commented 3 years ago

Good points. I haven't tested it during a print. I can move it to the fans section, I just went with a screen that had some empty spots in it. My concern is that it might not be obvious to find case light control in the fans section. Currently, the RGB LED controls are in Settings -> Machine. Wouldn't it make sense to have case light controls near here?

oldman4U commented 3 years ago

Don’t know how good your dev skills are and how much time you are willing to spend on it.

Makoto Schoppert notifications@github.com schrieb am Fr. 23. Okt. 2020 um 17:36:

Good points. I haven't tested it during a print. I can move it to the fans section, I just went with a screen that had some empty spots in it. My concern is that it might not be obvious to find case light control in the fans section. Currently, the RGB LED controls are in Settings -> Machine. Wouldn't it make sense to have case light controls near here?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/bigtreetech/BIGTREETECH-TouchScreenFirmware/issues/1194#issuecomment-715417413, or unsubscribe https://github.com/notifications/unsubscribe-auth/AM6XKZBZK2QVHEEWC7KFSADSMGPJBANCNFSM4S2MGK6Q .

mak0t0san commented 3 years ago

Been doing software development for about 20 years, but to be fair, not much in C, mostly C#, JavaScript/TypeScript, Java, Kotlin, SQL. I've been able to follow along with the code that's in place and use the same patterns.

oldman4U commented 3 years ago

20years sounds great😁. I have no case light but for me this would be the most logical place at the moment. Having it 3+ clicks away from main screen does not work I believe. Maybe I can help with icons and logical structure. Give me 24h. It is late here.

mak0t0san commented 3 years ago

To be honest, I don't change the case light brightness very often so 3+ clicks wouldn't be a deal breaker, but I agree that it is nice to reduce that for sure.
I've been able to use the RGB led icons (RGB_off to indicate off and RGB_white to indicate on). I've got the on/off toggle button working where it will turn the lights on and off, and even change the icon on the screen based on the state. Just need to add in the decrease/increase functionality.
I've got a Wifi module for it too that I need to setup and I need to ensure that sending commands via wifi will update the display on the TFT. I'll have more time this weekend to spend on it.

oldman4U commented 3 years ago

Chamber LED.zip

What do you think?

mak0t0san commented 3 years ago

I like it.

Here's a demo video of what I have in place so far: https://youtu.be/YUD3UnCrY9E

mak0t0san commented 3 years ago

I was noticing that '#pragma once' isn't used in the header files as include guards. Is there a reason for this? I find it easier and less error prone than using #ifndef and defining the header file name, and ensuring you have an end at the bottom of your header file.

oldman4U commented 3 years ago

I like it.

Here's a demo video of what I have in place so far:

https://youtu.be/YUD3UnCrY9E

This is really COOL!!! Goosebumps 😁

oldman4U commented 3 years ago

I was noticing that '#pragma once' isn't used in the header files as include guards. Is there a reason for this? I find it easier and less error prone than using #ifndef and defining the header file name, and ensuring you have an end at the bottom of your header file.

Please check #1170 where you will find a ticket exactly related to this.

In case you really want to add this to fan, I will make the Icons and buttons for you. This was just a test also for me.

In case it is ok for you, could you please check another ticket where a user asked for the ability to see the chamber temp alternating with the bed temp. Seems to be very similar of what you do and maybe we can make another user happy. This would be great. Thank you

PS. Gurmeet is the person who did most of the development here. He is very busy but a very very nice guy.

oldman4U commented 3 years ago

1182 is the ticket IF it is something you want to help

mak0t0san commented 3 years ago

So going back to the icon... looks like we'll need two different versions, and various different sizes for each. image

mak0t0san commented 3 years ago

And how to access it during print?

So playing around with during a print, the fan/heat settings don't really make sense and there's no room on those menus. I think the best spot for it is in the Machine Settings menu, which is also where the RGB LED button is. It would be the last item on the machine settings page, right before the back button. This is available during a print, and of course, when not printing.

oldman4U commented 3 years ago

Ok. So no direct access from main page. All you need is a button for Machine settings then.

Makoto Schoppert <@github.com> schrieb am Sa. 24. Okt. 2020 um 23:40:

And how to access it during print?

So playing around with during a print, the fan/heat settings don't really make sense and there's no room on those menus. I think the best spot for it is in the Machine Settings menu, which is also where the RGB LED button is. It would be the last item on the machine settings page, right before the back button. This is available during a print, and of course, when not printing.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/bigtreetech/BIGTREETECH-TouchScreenFirmware/issues/1194#issuecomment-716058884, or unsubscribe https://github.com/notifications/unsubscribe-auth/AM6XKZFDMKTU6QG5VUJV6SLSMNCUDANCNFSM4S2MGK6Q .

oldman4U commented 3 years ago

Forgot to mention that using the encoder is not a good idea. Same time you tune the light, you change the speed on Marlins main page which is active in the background.

mak0t0san commented 3 years ago

Forgot to mention that using the encoder is not a good idea. Same time you tune the light, you change the speed on Marlins main page which is active in the background.

I read your comment and thought you must be crazy for thinking that, as there are many menus that support using the encoder, but then I tried it out and sure enough, that is the case.
Although it only seems to be an issue if you are using the EXP1/EXP2 connectors for the classic Marlin interface. If there's only a serial connection, it doesn't seem to be an issue.

I'm assuming this is a known issue. I didn't see anything related to this in the list of open issues.

Honestly, I'm a bit conflicted as keeping the encoder functionality would be inline with the other menu screens and be consistent for having it there, but then again, as you pointed out if EXP1 cable is connected then the encoder functionality will unexpectedly affect print speed.

mak0t0san commented 3 years ago

I created a branch for the caselight changes. Unfortunately, I created the branch after my Junction Deviation changes (which still has an outstanding PR) so the caselight branch contains the changes for junction deviation as well. I would love to get a second set of eyes on the changes before I create a PR. I would either have to wait for #1192 to be merged in, or pull out the changes from that so that the PR stays on focused on the caselight feature. Here's a link to the changes: https://github.com/bigtreetech/BIGTREETECH-TouchScreenFirmware/compare/master...mak0t0san:caselight

I still need to add the string to the other languages and hopefully also include a new icon.

oldman4U commented 3 years ago

You will finally need 4 Icons.

I am trying to organise the original one. So you are not going to implement this as a second option on the main screen, right?

Standard Button Size

oldman4U commented 3 years ago

See #1203

mak0t0san commented 3 years ago

You will finally need 4 Icons.

I am trying to organise the original one. So you are not going to implement this as a second option on the main screen, right?

Standard Button Size

I am not sure where else this could fit. It seems to make the most sense to put it in the Machine Settings screen which is where the RGB settings button is. The Machine Settings screen is easy to get to mid-print as well. While I don't love the extra clicks needed to get there, I don't think the case light is something that is adjusted very frequently, at least that's true for myself.

To get there currently, have to click on: Menu -> Settings -> Machine -> Case light Mid print: More -> Machine -> Case light

I'm open to other ideas and took into consideration of putting into the "Heat/fan" menu, but that menu isn't available during print.

oldman4U commented 3 years ago

Lets stay with your solution for now.

You get the buttons within 24h. Any background color you would prefer?

mak0t0san commented 3 years ago

Lets stay with your solution for now.

You get the buttons within 24h. Any background color you would prefer?

I have no color preference. I'm not much of a graphic designer personally, but as long as it's consistent with the current themes and fit in, I think it will be fine.

oldman4U commented 3 years ago

Chamber Icons.zip

I could not find a scheme the colors are following, so I used the blue. Please check the attached Icons and let me know.

Thank you

mak0t0san commented 3 years ago

The icons ended up crashing the TFT upon loading them up. Turns out they had a depth of 32 bit (which explains why the file sizes were bigger compared to the other bmps). I used ImageMagick and set alpha to false and that got it working and the file sizes are consistent with the other bmps.

Attached is what it looks like on a TFT35 and a TFT24. PR will be created shortly. TFT35 TFT24

Also, here are the updated bmps in case you wanted them: Case Light Icons.zip

oldman4U commented 3 years ago

Hi. Saw you just pushed the PR, cool!!

Sorry that the buttons were wrong. Making BMPs on a Mac is not an easy task...

Anything else I can help you with?

mak0t0san commented 3 years ago

Thanks for your help! I think everything is good for now. Really appreciate everything!

oldman4U commented 3 years ago

Pleasure was on my side. Thank you

oldman4U commented 3 years ago

Hi. Hope you are fine.

Would you please be so kind to close this ticket now that your PR has been merged.

Thank you

github-actions[bot] commented 6 months ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.