MarlinFirmware / Marlin

Marlin is an optimized firmware for RepRap 3D printers based on the Arduino platform. Many commercial 3D printers come with Marlin installed. Check with your vendor if you need source code for your specific machine.
https://marlinfw.org
GNU General Public License v3.0
16.32k stars 19.25k forks source link

[BUG] LED control in encoder.cpp - invalid narrowing conversion #27356

Open classicrocker883 opened 3 months ago

classicrocker883 commented 3 months ago

Did you test the latest bugfix-2.1.x code?

Yes, and the problem still exists.

Bug Description

invalid narrowing conversion from "int" to "unsigned char"

image

It is apparent that R and G are swapped, generally it should follow RGB, not GRB, and also you see G (or g) shifts 16 bits, not 8 which it is supposed to, as for R (or r) it shifts 8. so basically, these are backwards. I was messing around and saw LED_Action() commented out and wondered why

<div class="highlight highlight-source-yacc notranslate position-relative overflow-auto" dir="auto" data-snippet-clipboard-copy-content=" #if PIN_EXISTS(LCD_LED) //LED_Action();

endif">
      #if PIN_EXISTS(LCD_LED)

    <span class="pl-c"><span class="pl-c">//</span>LED_Action();</span>
  #<span class="pl-k">endif</span></pre></div>

so I enabled that pin, and stumbled upon this error.

one possible fix is

-    struct { uint8_t g, r, b; } led_data[LED_NUM];
     for (uint8_t i = 0; i < LED_NUM; i++) {
       switch (RGB_Scale) {
         case RGB_SCALE_R10_G7_B5:
-           led_data[i] = { luminance * 7/10, luminance * 10/10, luminance * 5/10 };

+    struct { uint32_t g, r, b; } led_data[LED_NUM];
     for (uint8_t i = 0; i < LED_NUM; i++) {
       switch (RGB_Scale) {
         case RGB_SCALE_R10_G7_B5:
+          led_data[i] = { luminance * (u_int)7/10, luminance * (u_int)10/10, luminance * (u_int)5/10 };

I would also like to point out this code:

  // LED initialization
  void LED_Configuration() {
    SET_OUTPUT(LCD_LED_PIN);
  }

It is unused, so chime in if anyone has experience in this area so I can test this properly

Bug Timeline

opened #26552 but the issue was closed without the PR I created being merged

Expected behavior

should not give error for invalid conversion

Actual behavior

gives error for invalid conversion

Steps to Reproduce

  1. make sure you define HAS_DWIN_E3V2 -- i.e. DWIN_LCD_PROUI
  2. inside pins_(YOUR_BOARD).h -- i.e. Marlin\src\pins\stm32f1\pins_CREALITY_V4.h
  3. add #define LCD_LED_PIN EXP3_02_PIN -- or any pin
  4. go to encoder.cpp and you will see the error

Version of Marlin Firmware

bugfix-2.1.x

Printer model

Aquila

Electronics

No response

LCD/Controller

No response

Other add-ons

No response

Bed Leveling

None

Your Slicer

None

Host Software

None

Don't forget to include

Additional information & file uploads

No response

I may be able to test this at some point using #26946

github-actions[bot] commented 1 week ago

Greetings from the Marlin AutoBot! This issue has had no activity for the last 90 days. Do you still see this issue with the latest bugfix-2.1.x code? Please add a reply within 14 days or this issue will be automatically closed. To keep a confirmed issue open we can also add a "Bug: Confirmed" tag.

Disclaimer: This is an open community project with lots of activity and limited resources. The main project contributors will do a bug sweep ahead of the next release, but any skilled member of the community may jump in at any time to fix this issue. That can take a while depending on our busy lives so please be patient, and take advantage of other resources such as the MarlinFirmware Discord to help solve the issue.