Extend RGB LED with Printer Events #6240

thinkyhead · 2017-04-04T21:30:54Z

Rebase and squash of #5395 by @boweeble

RGB_LED_STRIP was not needed as a separate feature. The only difference was that the led strip "required" PWM pins, while RGB_LED would work with either PWM or 5V digital.
Adjustments have been made from a suggestion by @Tannoo that color transition (while waiting for heating) proportionally using the current temperature when the loop starts as the low bound.

It is still possible to add software PWM for those cases where one or more pins isn't hardware-PWM-capable, but we need a way to determine at compile-time that a pin is PWM capable (and available for use because it's not associated with a timer).

thinkyhead · 2017-04-04T23:22:49Z

Marlin/Marlin_main.cpp

-          r = 255;
-          g = 0;
-          set_led_color(r, g, b);
+          set_led_color(255, 0, 0);


@boweeble I assume this is the correct fix?

ghost · 2017-04-05T03:20:13Z

The RGB strip does not have to be pwm pins. The TIP's don't care. Can be digital, but transitions won't happen.

ghost · 2017-04-05T15:26:00Z

I just got through testing this.

Brings back memories of the issues I worked through on this.

thinkyhead · 2017-04-06T02:27:52Z

Right, PWM is truly optional. I'll modify the notes.

Tannoo · 2017-04-07T04:52:12Z

I'm curious. Why submit a new feature that needs work when getting ready for a release?
I had this completely working with the current RCBugFix (and still do).

thinkyhead · 2017-04-08T22:34:40Z

@Tannoo I was using this as an example while working on a video explaining how to use git for the developers of Marlin. So, then I've been trying to polish it up since. And actually, it doesn't amount to a new feature after all. It's ending up being more like just changing the notes on the existing RGB_LED feature. And, well, I guess there is PRINTER_EVENT_LEDS where it changes the color of the LED at certain points….

I don't know if this would get merged before the release. Do you think it looks innocuous enough to merge?

How's things with you? Working on anything interesting lately?

Tannoo · 2017-04-08T23:17:35Z

I would not merge this as it is incomplete and doesn't doo all of what's advertised.

I am ok. Currently trying to track down something I noticed this afternoon on my display. Not sure if it's something I did or not.
The X and Y are not reporting the accurate position anymore. Used to report X.XX.. Now, it's just X. Also, when going above 99.9, it shows 00.0. At 110.0, it shows 10.0.

I've gone back several of my backups ago and it's still the same. So, I'm not sure when that broke.

Tannoo · 2017-04-08T23:20:16Z

I will dig into it more in the am.

thinkyhead · 2017-04-08T23:30:33Z

I would not merge this as it is incomplete and doesn't doo all of what's advertised.

I generally agree. All it now amounts to is PRINTER_EVENT_LEDS. I will update the title to reflect that change.

This PR doesn't need to worry about PWM versus non-PWM, since both will work with the right pins. Obviously we can't prevent someone hooking up an LED strip wrong (without power MOSFETs) so I think the warnings are enough in support of LED Strips. The only thing really left to do with this is to make sure the PRINTER_EVENT_LEDS color-change points make sense. Obviously not a big priority. I will definitely use this for one of my projects, though, and may add more color-changing events for that.

thinkyhead · 2017-04-08T23:31:36Z

The X and Y are not reporting the accurate position anymore. Used to report X.XX.. Now, it's just X. Also, when going above 99.9, it shows 00.0. At 110.0, it shows 10.0.

I'll flash the latest RCBugFix on my machine and take a look. I assume you're using a graphical display…? I'll be testing with character.

Tannoo · 2017-04-08T23:40:26Z

Yes. Reprap full graphic.

Tannoo · 2017-04-08T23:44:49Z

I already have the led things working on mine. Printer events for homing, leveling, printing, filament change events, and done homing.

I have tested it with a single rgb led, an rgb strip, an rgbw strip, and an addressable rgb strip.

The only thing I couldn't test yet is the blinkm.

thinkyhead · 2017-04-08T23:47:38Z

Patched to correct the "violet-to-red" color transition.
Moved cleanup/formatting commits over to the existing cleanup PR

Tannoo · 2017-04-11T11:37:56Z

float starting_temp_b = (thermalManager.degBed()) - 2;

I'm not sure if I need the -2 anymore.

thinkyhead · 2017-04-11T11:39:59Z

This sets the initial starting color that otherwise might not be set.

The initial color setting in this PR is guaranteed by the initialized values of old_red and old_blue being highly unlikely to match the first calculated values of red and blue.

Tannoo · 2017-04-11T11:46:54Z

You know, I may have just run into pure luck that it's been working for me. lol

I will change it to something that is guaranteed.

thinkyhead · 2017-04-11T12:00:20Z

You know, I may have just run into pure luck that it's been working for me.

That was my working assumption. 😉

Tannoo · 2017-04-11T13:15:45Z

I will have to test what you have done here without my minglings.

The initial color setting in this PR is guaranteed by the initialized values of old_red and old_blue being highly unlikely to match the first calculated values of red and blue.

I have been trying to implement this into mine, and all I get is a slower flashing of red and purple.
I've changed (r == starting_temp_b) to (r == 0) and it doesn't turn on for quite a while. It doesn't make sense.

Tannoo · 2017-04-11T13:17:04Z

Marlin/Marlin_main.cpp

@@ -6195,11 +6251,16 @@ inline void gcode_M109() {

    target_extruder = active_extruder; // for print_heaterstates

+    #if ENABLED(PRINTER_EVENT_LEDS)
+      const float start_temp = thermalManager.degTargetBed();
+      uint8_t old_red = 255;


const float start_temp = thermalManager.degBed();
const uint8_t old_red = 0;

Ah, right… thermalManager.degBed is better!

Since old_red is altered below, it can't be const. And it should be 255 to start, because this ensures the color will be set on the first iteration (because the initial red will be closer to 0).

Tannoo · 2017-04-11T13:17:40Z

Marlin/Marlin_main.cpp

  bool wants_to_cool = false;
  wait_for_heatup = true;
  millis_t now, next_temp_ms = 0, next_cool_check_ms = 0;

  KEEPALIVE_STATE(NOT_BUSY);

+  #if ENABLED(PRINTER_EVENT_LEDS)
+    const float start_temp = thermalManager.degTargetHotend(target_extruder);
+    uint8_t old_blue = 0;


const float start_temp = thermalManager.degHotend(target_extruder);
const uint8_t old_blue = 255

Ah, right… thermalManager.degHotend is better!

Since old_blue is altered below, it can't be const. And it should be 0 to start, because this ensures the color will be set on the first iteration (because the initial blue will be closer to 255).

Tannoo · 2017-04-11T16:27:01Z

I got a copy of your repo and got this to compile, then got the transitions working.
I cannot create a PR against your branch, it gives me "validation error". So... maybe merge this and I'll put it against RCBugFix.

Tannoo · 2017-04-11T16:31:06Z

Simple changes and not that many to make it work. I'll just comment them in the code.

Tannoo · 2017-04-11T16:36:52Z

Marlin/Marlin_main.cpp

+        const uint8_t blue = map(constrain(temp, start_temp, target_temp), start_temp, target_temp, 255, 0);
+        if (blue != old_blue) set_led_color(255, 0, (old_blue = blue));
+      }
+    #endif



#if ENABLED(PRINTER_EVENT_LEDS) // Gradually change LED strip from violet to red as nozzle heats up if (wait_for_heatup && !wants_to_cool) { const uint8_t blue = map(constrain(temp, start_temp, target_temp), start_temp, target_temp, 255, 0); if (blue == old_blue) set_led_color(255, 0, 255); //Purple to start if (blue != old_blue) set_led_color(255, 0, blue); //Start transitioning to Red } #endif

I'd generally reduce those two lines down to one:

set_led_color(255, 0, blue == old_blue ? 255 : blue);

…but they are incorrect. First of all, old_blue is not being set to blue when it differs. Secondly, with that correction made, blue == old_blue will evaluate true most of the time.

Tannoo · 2017-04-11T16:38:03Z

Marlin/Marlin_main.cpp

+          if (red != old_red) set_led_color((old_red = red), 0, 255);
+        }
+      }
+      #endif


#if ENABLED(PRINTER_EVENT_LEDS) // Gradually change LED strip from blue to violet as bed heats up if (wait_for_heatup && !wants_to_cool) { const uint8_t red = map(constrain(temp, start_temp, target_temp), start_temp, target_temp, 0, 255); if (red == old_red) set_led_color(0, 0, 255); //Blue to start if (red != old_red) set_led_color(red, 0, 255); //Start transitioning to Purple } #endif

Tannoo · 2017-04-11T16:40:05Z

It works fine with those changes.

thinkyhead · 2017-04-11T17:12:34Z

I cannot create a PR against your branch

Trying from the web interface or from Github Desktop? It should be easier from the web interface. If you open your fork's page, the link to create a PR will be at the top.

thinkyhead · 2017-04-11T17:25:44Z

It works fine with those changes.

Only by kismet. The code you pasted will simply always pass the test red != old_red once red changes, so it is merely setting the color on every loop once the value of red changes. This is because you are not updating old_red. The purpose of the old_red and old_blue values is to track when red or blue has changed, so the LED will only be updated as necessary. By initializing old_red and old_blue to the final values of red and blue, we ensure the first test passes and the color is always set on the first iteration. This is a common programming pattern.

Yep, I do dream in code. I started programming in 1978 in Sinclair BASIC. By 1984 I was programming 6502 Assembler. In 1988 I was programming 680x0 Assembler, writing Amiga games. But I must admit, I didn't start programming in C++ until around 1997. Of course, now I eat new languages for breakfast. 😜

thinkyhead · 2017-04-11T17:33:33Z

I presume that for an RGBW LED we just need to add one more pin and treat all gray values as varying intensities of white. For example, if the color given is 128, 128, 128 the color output should be R=0 G=0 B=0 W=128. Unless we want to mix other colors with white….

thinkyhead · 2017-04-11T19:27:11Z

Patched to support RGBW LEDs.

Tannoo · 2017-04-12T00:22:57Z

I presume that for an RGBW LED we just need to add one more pin and treat all gray values as white. For example, if the color given is 128, 128, 128 the color output should be R=0 G=0 B=0 W=128.

That's the idea. set_led_color(r, g, b, w).

Tannoo · 2017-04-12T01:34:04Z

It's back to flickering again....WTH?

I think it's due to your code is too efficient and it's executing too fast.

nesalkiN · 2017-04-13T13:41:36Z

So, no ws2812b support as of now? :'(

Tannoo · 2017-04-13T15:14:40Z

Not officially, yet.

Although, my branch has it currently working.

thinkyhead added PR: New Feature C: LCD & Controllers labels Apr 4, 2017

thinkyhead force-pushed the rebuild_rgb_led branch 3 times, most recently from 84b9c24 to 2416037 Compare April 4, 2017 23:22

thinkyhead commented Apr 4, 2017

View reviewed changes

thinkyhead force-pushed the rebuild_rgb_led branch 2 times, most recently from 90a0b3c to 684384c Compare April 4, 2017 23:49

thinkyhead added the Needs: Work More work is needed label Apr 5, 2017

thinkyhead force-pushed the rebuild_rgb_led branch 2 times, most recently from f82ebf0 to b396486 Compare April 5, 2017 00:44

thinkyhead force-pushed the RCBugFix branch from db2d3c0 to 52978e5 Compare April 6, 2017 00:07

thinkyhead force-pushed the rebuild_rgb_led branch from b396486 to daae9df Compare April 7, 2017 16:12

thinkyhead force-pushed the rebuild_rgb_led branch from daae9df to 0fa0fab Compare April 8, 2017 23:44

thinkyhead force-pushed the rebuild_rgb_led branch from 0fa0fab to bd96806 Compare April 8, 2017 23:45

thinkyhead force-pushed the rebuild_rgb_led branch 3 times, most recently from b04afcb to b79bafe Compare April 8, 2017 23:57

thinkyhead mentioned this pull request Apr 11, 2017

RGB_Strip, RGBW_Strip, and LEDSTRIP (WS2812B) Support #5633

Closed

Tannoo reviewed Apr 11, 2017

View reviewed changes

thinkyhead added 5 commits April 11, 2017 12:15

Use multiply for delta probe constraint

a2cb0a3

2 is DXC_DUPLICATION_MODE

53e6c33

Add HAS_RESUME_CONTINUE shorthand macro

36bda2b

theTarget => target_temp

c7063eb

Extend RGB LED support, adding Printer Events

e7746ff

thinkyhead force-pushed the rebuild_rgb_led branch from 1cac42b to e7746ff Compare April 11, 2017 17:15

Add support for an RGBW LED

dac21ec

thinkyhead merged commit 6d5400d into MarlinFirmware:RCBugFix Apr 11, 2017

thinkyhead deleted the rebuild_rgb_led branch April 11, 2017 20:17

Extend RGB LED with Printer Events #6240

Extend RGB LED with Printer Events #6240

Conversation

thinkyhead commented Apr 4, 2017 • edited Loading

thinkyhead Apr 4, 2017

Choose a reason for hiding this comment

ghost commented Apr 5, 2017

ghost commented Apr 5, 2017

thinkyhead commented Apr 6, 2017

Tannoo commented Apr 7, 2017

thinkyhead commented Apr 8, 2017 • edited Loading

Tannoo commented Apr 8, 2017

Tannoo commented Apr 8, 2017

thinkyhead commented Apr 8, 2017 • edited Loading

thinkyhead commented Apr 8, 2017

Tannoo commented Apr 8, 2017

Tannoo commented Apr 8, 2017

thinkyhead commented Apr 8, 2017 • edited Loading

Tannoo commented Apr 11, 2017 • edited Loading

thinkyhead commented Apr 11, 2017 • edited Loading

Tannoo commented Apr 11, 2017

thinkyhead commented Apr 11, 2017 • edited Loading

Tannoo commented Apr 11, 2017

Tannoo Apr 11, 2017 • edited Loading

Choose a reason for hiding this comment

thinkyhead Apr 11, 2017 • edited Loading

Choose a reason for hiding this comment

Tannoo Apr 11, 2017 • edited Loading

Choose a reason for hiding this comment

thinkyhead Apr 11, 2017 • edited Loading

Choose a reason for hiding this comment

Tannoo commented Apr 11, 2017 • edited Loading

Tannoo commented Apr 11, 2017

Tannoo Apr 11, 2017

Choose a reason for hiding this comment

thinkyhead Apr 11, 2017

Choose a reason for hiding this comment

Tannoo Apr 11, 2017

Choose a reason for hiding this comment

Tannoo commented Apr 11, 2017

thinkyhead commented Apr 11, 2017

thinkyhead commented Apr 11, 2017 • edited Loading

thinkyhead commented Apr 11, 2017 • edited Loading

thinkyhead commented Apr 11, 2017 • edited Loading

Tannoo commented Apr 12, 2017

Tannoo commented Apr 12, 2017 • edited Loading

nesalkiN commented Apr 13, 2017

Tannoo commented Apr 13, 2017

thinkyhead commented Apr 4, 2017 •

edited

Loading

thinkyhead commented Apr 8, 2017 •

edited

Loading

thinkyhead commented Apr 8, 2017 •

edited

Loading

thinkyhead commented Apr 8, 2017 •

edited

Loading

Tannoo commented Apr 11, 2017 •

edited

Loading

thinkyhead commented Apr 11, 2017 •

edited

Loading

thinkyhead commented Apr 11, 2017 •

edited

Loading

Tannoo Apr 11, 2017 •

edited

Loading

thinkyhead Apr 11, 2017 •

edited

Loading

Tannoo Apr 11, 2017 •

edited

Loading

thinkyhead Apr 11, 2017 •

edited

Loading

Tannoo commented Apr 11, 2017 •

edited

Loading

thinkyhead commented Apr 11, 2017 •

edited

Loading

thinkyhead commented Apr 11, 2017 •

edited

Loading

thinkyhead commented Apr 11, 2017 •

edited

Loading

Tannoo commented Apr 12, 2017 •

edited

Loading