Array troubles

[Tannoo]

@Roxy-3D,

I have an issue with this and unsure what to do.

    void _lcd_set_led_color() {
      const char *_colors[10] = {
        MSG_LCD_RED,         // 0  -- color_number
        MSG_LCD_GREEN,       // 1
        MSG_LCD_BLUE,        // 2
        MSG_LCD_AQUA,        // 3
        MSG_LCD_YELLOW,      // 4
        MSG_LCD_PURPLE,      // 5
        MSG_LCD_PINK,        // 6
        MSG_LCD_AMBER,       // 7
        MSG_LCD_BLACKLIGHT,  // 8
        MSG_LCD_WATERMELON   // 9
      };

      START_MENU();
      MENU_ITEM(back, MSG_LED_LIGHTING, lcd_main_menu);
      MENU_ITEM_EDIT_CALLBACK(int3, _colors[color_number], &color_number, 0, 9, _color_select);
      MENU_ITEM(submenu, MSG_LCD_RGB_EDIT, _lcd_set_rgb_color);
      MENU_ITEM(function, MSG_LCD_COLOR_RESET, _lcd_set_led_color_reset);
      END_MENU();
    }

This compiles and works just fine with the Re-ARM, I do see the linter errors are exactly the same as AVR. The compiler errors in the Arduino IDE for the Mega2560:

sketch\ultralcd.cpp: In function 'void _lcd_set_led_color()':

ultralcd.cpp:305: error: initializer fails to determine size of '__c'

   #define MENU_ITEM_EDIT_CALLBACK(type, label, ...) MENU_ITEM(setting_edit_callback_ ## type, label, PSTR(label), ## __VA_ARGS__)

                                                                                                      ^

sketch\ultralcd.cpp:253:30: note: in definition of macro '_MENU_ITEM_PART_2'

         menu_action_ ## TYPE(__VA_ARGS__); \

                              ^

sketch\ultralcd.cpp:305:53: note: in expansion of macro 'MENU_ITEM'

   #define MENU_ITEM_EDIT_CALLBACK(type, label, ...) MENU_ITEM(setting_edit_callback_ ## type, label, PSTR(label), ## __VA_ARGS__)

                                                     ^

sketch\ultralcd.cpp:3068:7: note: in expansion of macro 'MENU_ITEM_EDIT_CALLBACK'

       MENU_ITEM_EDIT_CALLBACK(int3, _colors[color_number], &color_number, 0, 9, _color_select);

       ^

ultralcd.cpp:305: error: array must be initialized with a brace-enclosed initializer

   #define MENU_ITEM_EDIT_CALLBACK(type, label, ...) MENU_ITEM(setting_edit_callback_ ## type, label, PSTR(label), ## __VA_ARGS__)

                                                                                                      ^

sketch\ultralcd.cpp:253:30: note: in definition of macro '_MENU_ITEM_PART_2'

         menu_action_ ## TYPE(__VA_ARGS__); \

                              ^

sketch\ultralcd.cpp:305:53: note: in expansion of macro 'MENU_ITEM'

   #define MENU_ITEM_EDIT_CALLBACK(type, label, ...) MENU_ITEM(setting_edit_callback_ ## type, label, PSTR(label), ## __VA_ARGS__)

                                                     ^

sketch\ultralcd.cpp:3068:7: note: in expansion of macro 'MENU_ITEM_EDIT_CALLBACK'

       MENU_ITEM_EDIT_CALLBACK(int3, _colors[color_number], &color_number, 0, 9, _color_select);

       ^

In file included from C:\Users\JASON\AppData\Local\Arduino15\packages\arduino\hardware\avr\1.6.19\cores\arduino/Arduino.h:28:0,

                 from sketch\src/HAL/HAL_AVR/HAL_AVR.h:38,

                 from sketch\src/HAL/HAL.h:81,

                 from sketch\MarlinConfig.h:27,

                 from sketch\Marlin.h:31,

                 from sketch\ultralcd.h:26,

                 from sketch\ultralcd.cpp:23:

ultralcd.cpp:257: error: initializer fails to determine size of '__c'

         lcd_implementation_drawmenu_ ## TYPE(encoderLine == _thisItemNr, _lcdLineNr, PSTR(LABEL), ## __VA_ARGS__); \

                                                                                      ^

sketch\ultralcd.cpp:264:7: note: in expansion of macro '_MENU_ITEM_PART_2'

       _MENU_ITEM_PART_2(TYPE, LABEL, ## __VA_ARGS__); \

       ^

sketch\ultralcd.cpp:305:53: note: in expansion of macro 'MENU_ITEM'

   #define MENU_ITEM_EDIT_CALLBACK(type, label, ...) MENU_ITEM(setting_edit_callback_ ## type, label, PSTR(label), ## __VA_ARGS__)

                                                     ^

sketch\ultralcd.cpp:3068:7: note: in expansion of macro 'MENU_ITEM_EDIT_CALLBACK'

       MENU_ITEM_EDIT_CALLBACK(int3, _colors[color_number], &color_number, 0, 9, _color_select);

       ^

ultralcd.cpp:257: error: array must be initialized with a brace-enclosed initializer

         lcd_implementation_drawmenu_ ## TYPE(encoderLine == _thisItemNr, _lcdLineNr, PSTR(LABEL), ## __VA_ARGS__); \

                                                                                      ^

sketch\ultralcd.cpp:264:7: note: in expansion of macro '_MENU_ITEM_PART_2'

       _MENU_ITEM_PART_2(TYPE, LABEL, ## __VA_ARGS__); \

       ^

sketch\ultralcd.cpp:305:53: note: in expansion of macro 'MENU_ITEM'

   #define MENU_ITEM_EDIT_CALLBACK(type, label, ...) MENU_ITEM(setting_edit_callback_ ## type, label, PSTR(label), ## __VA_ARGS__)

                                                     ^

sketch\ultralcd.cpp:3068:7: note: in expansion of macro 'MENU_ITEM_EDIT_CALLBACK'

       MENU_ITEM_EDIT_CALLBACK(int3, _colors[color_number], &color_number, 0, 9, _color_select);

       ^

ultralcd.cpp:305: error: initializer fails to determine size of '__c'

   #define MENU_ITEM_EDIT_CALLBACK(type, label, ...) MENU_ITEM(setting_edit_callback_ ## type, label, PSTR(label), ## __VA_ARGS__)

                                                                                                      ^

sketch\ultralcd.cpp:264:7: note: in expansion of macro '_MENU_ITEM_PART_2'

       _MENU_ITEM_PART_2(TYPE, LABEL, ## __VA_ARGS__); \

       ^

sketch\ultralcd.cpp:305:53: note: in expansion of macro 'MENU_ITEM'

   #define MENU_ITEM_EDIT_CALLBACK(type, label, ...) MENU_ITEM(setting_edit_callback_ ## type, label, PSTR(label), ## __VA_ARGS__)

                                                     ^

sketch\ultralcd.cpp:3068:7: note: in expansion of macro 'MENU_ITEM_EDIT_CALLBACK'

       MENU_ITEM_EDIT_CALLBACK(int3, _colors[color_number], &color_number, 0, 9, _color_select);

       ^

ultralcd.cpp:305: error: array must be initialized with a brace-enclosed initializer

   #define MENU_ITEM_EDIT_CALLBACK(type, label, ...) MENU_ITEM(setting_edit_callback_ ## type, label, PSTR(label), ## __VA_ARGS__)

                                                                                                      ^

sketch\ultralcd.cpp:264:7: note: in expansion of macro '_MENU_ITEM_PART_2'

       _MENU_ITEM_PART_2(TYPE, LABEL, ## __VA_ARGS__); \

       ^

sketch\ultralcd.cpp:305:53: note: in expansion of macro 'MENU_ITEM'

   #define MENU_ITEM_EDIT_CALLBACK(type, label, ...) MENU_ITEM(setting_edit_callback_ ## type, label, PSTR(label), ## __VA_ARGS__)

                                                     ^

sketch\ultralcd.cpp:3068:7: note: in expansion of macro 'MENU_ITEM_EDIT_CALLBACK'

       MENU_ITEM_EDIT_CALLBACK(int3, _colors[color_number], &color_number, 0, 9, _color_select);

       ^

exit status 1
initializer fails to determine size of '__c'

[Roxy-3D]

initializer fails to determine size of '__c'

Can you point me at the branch where you are seeing this? What file is c declared in? I'm guessing c is an enumerated type?

[Tannoo]

https://github.com/Tannoo/Marlin/tree/Re-Arm_bugfixes

This is adding an LCD menu to control RGBs.

[Tannoo]

That is the branch that I have been playing with on the Re-ARM. It works on the Re-ARM, but I was going to load it on the Mega (my printer)... so, I copied this clone to another directory and changed the board and AVR will not get past this one hiccup.

If I can get past this, the Linter errors might disappear also.

[Roxy-3D]

This is very strange. I searched the entire code base for c and it doesn't show up. There are cplusplus and other things with c as the start of the symbol. But I don't see c anywhere. But maybe I'm looking at the wrong branch. I searched for MSG_LCD_RED and that isn't in the branch you linked to up above either.

[Tannoo]

hmmm... hold on.

[Tannoo]

Well... __c is in a macro somewhere.

OH...I haven't committed these changes to that branch. I'll create another branch with all my changes.

[Tannoo]

Okay, it should be here:

https://github.com/Tannoo/Marlin/tree/My_Re-ARM

[Roxy-3D]

I lost internet service before you posted the link. I just got service back again. Do you still have the problem?

[Tannoo]

Yes :(

[Roxy-3D]

I see why you were so confused and frustrated. I don't have the full answer for you. But I have some valuable clues. When you look at MENU_ITEM_EDIT_CALLBACK() usage in the rest of ultralcd.cpp you are the only place where a variable is being passed into it for the 'label'. I even tried a simple construct like

char *ptr;
ptr = "Test";
MENU_ITEM_EDIT_CALLBACK(int3, ptr, &color_number, 0, 10, _color_select);

and that failed. If that had worked, I would have suggested doing

char *ptr;
ptr = _color[color_number];
MENU_ITEM_EDIT_CALLBACK(int3, ptr, &color_number, 0, 10, _color_select);

What I do know is it has something to do with the PSTR() macro trying to force stuff into program memory. What you really are doing with that _color[color_number] term is giving the macro a pointer to RAM memory and it has no way to force the contents into program memory with the PSTR() macro. I have a temporary alternative but it is clunky.

    void _lcd_set_led_color() {
      START_MENU();
      MENU_ITEM(back, MSG_LED_LIGHTING, lcd_main_menu);
      switch (color_number) {
        case 0: MENU_ITEM_EDIT_CALLBACK(int3, MSG_LCD_CHOOSE, &color_number, 0, 10, _color_select);
          break;
        case 1: MENU_ITEM_EDIT_CALLBACK(int3, MSG_LCD_RED, &color_number, 0, 10, _color_select);
          break;
        case 2: MENU_ITEM_EDIT_CALLBACK(int3, MSG_LCD_GREEN, &color_number, 0, 10, _color_select);
          break;
        case 3: MENU_ITEM_EDIT_CALLBACK(int3, MSG_LCD_BLUE, &color_number, 0, 10, _color_select);
          break;
        case 4: MENU_ITEM_EDIT_CALLBACK(int3, MSG_LCD_AQUA, &color_number, 0, 10, _color_select);
          break;
        case 5: MENU_ITEM_EDIT_CALLBACK(int3, MSG_LCD_YELLOW, &color_number, 0, 10, _color_select);
          break;
        case 6: MENU_ITEM_EDIT_CALLBACK(int3, MSG_LCD_PURPLE, &color_number, 0, 10, _color_select);
          break;
        case 7: MENU_ITEM_EDIT_CALLBACK(int3, MSG_LCD_PINK, &color_number, 0, 10, _color_select);
          break;
        case 8: MENU_ITEM_EDIT_CALLBACK(int3, MSG_LCD_AMBER, &color_number, 0, 10, _color_select);
          break;
        case 9: MENU_ITEM_EDIT_CALLBACK(int3, MSG_LCD_BLACKLIGHT, &color_number, 0, 10, _color_select);
          break;
        case 10: MENU_ITEM_EDIT_CALLBACK(int3, MSG_LCD_WATERMELON, &color_number, 0, 10, _color_select);
          break;
      }
      MENU_ITEM(submenu, MSG_LCD_RGB_EDIT, _lcd_set_rgb_color);
      MENU_ITEM(function, MSG_LCD_COLOR_RESET, _lcd_set_led_color_reset);
      END_MENU();
    }

That compiles clean. But what I think I would do is abandon the use of the code generator macro for MENU_ITEM_EDIT_CALLBACK(); and manually expand that macro into a function. And then call that code in a similar way

  char *ptr;
  ptr = _color[color_number];
  My_manually_expanded_callback_macro(int3, ptr, &color_number, 0, 10, _color_select);

Because that macro stuff is cryptic as shit. If you manually expand the macro, we can play with each term and work through the issues. The biggest downside is if somebody changes the MENU_ITEM_EDIT_CALLBACK() macro, your new code will get left behind.

[Tannoo]

Yeah... thank you for looking at it. I will play with it.

Another note... I found and corrected the mean_fade_height being set backwards in the lcd menu and also it not being displayed in the G29 W command.

I am on nights again starting tonight and leaving for work soon.

In ultralcd.cpp, ~ line 1900:

const int ind = ubl_height_amount < 0 ? 6 : 7;

Needs to be changed to:

const int ind = ubl_height_amount > 0 ? 6 : 7;

Also, in ubl_G29.cpp:

I added:

find_mean_mesh_height();

Just after the report of the leveling fade height.

[Roxy-3D]

Are you going to generate a PR for that (against bugfix-v1.2.x) ? Or are you telling me this so do it?

[Tannoo]

Also, doing some quick tests, it doesn't look like G29 P6 is changing the mean_mesh_height.

[Tannoo]

I can, but not today.

[Roxy-3D]

The G29 P6 code looks like this:

  void unified_bed_leveling::shift_mesh_height() {
    for (uint8_t x = 0; x < GRID_MAX_POINTS_X; x++)
      for (uint8_t y = 0; y < GRID_MAX_POINTS_Y; y++)
        if (!isnan(z_values[x][y]))
          z_values[x][y] += g29_constant;
  }

And in the parser... it should be setting the g29_constant

    // Set global 'C' flag and its value
    if ((g29_c_flag = parser.seen('C')))
      g29_constant = parser.value_float();

Maybe there is a bug in the parser code????

[Tannoo]

Yeah, I was looking at that, but I have to leave now.

[Tannoo]

I was playing on the Re-ARM... not AVR. In case that makes a difference. I've haven't really tested this on AVR.

[Roxy-3D]

At least on AVR... G29 P6 C ?.?? seems to work

G29 T SENDING:G29 T Bed Topography Report: (0,9) (9,9) (45,375) (375,375) -0.075 -0.043 -0.060 -0.083 -0.100 -0.120 -0.170 -0.227 -0.273 -0.312

-0.118 -0.123 -0.107 -0.142 -0.145 -0.160 -0.212 -0.232 -0.345 -0.352

-0.147 -0.115 -0.135 -0.147 -0.157 -0.190 -0.223 -0.247 -0.300 -0.390

-0.150 -0.123 -0.095 -0.115 -0.150 -0.140 -0.177 -0.190 -0.270 -0.300

-0.083 -0.080 -0.040 -0.032 -0.085 [-0.080] -0.095 -0.125 -0.197 -0.220

-0.030 -0.030 0.027 0.020 -0.020 0.000 -0.030 -0.043 -0.102 -0.123

0.048 0.070 0.102 0.112 0.095 0.100 0.070 0.080 0.005 -0.027

0.182 0.140 0.230 0.210 0.220 0.217 0.200 0.157 0.175 0.093

0.265 0.362 0.348 0.355 0.345 0.350 0.357 0.362 0.290 0.292

0.452 0.518 0.555 0.585 0.580 0.573 0.550 0.555 0.507 0.435 (45,45) (375,45) (0,0) (9,0) G29 P6 C 5.00 SENDING:G29 P6 C 5.00 M114 SENDING:M114 X:223.00 Y:224.00 Z:16.03 E:0.00 Count X:17840 Y:17920 Z:6411 G29 T SENDING:G29 T Bed Topography Report: (0,9) (9,9) (45,375) (375,375) 4.925 4.958 4.940 4.918 4.900 4.880 4.830 4.773 4.728 4.688

4.883 4.878 4.893 4.858 4.855 4.840 4.788 4.768 4.655 4.648

4.853 4.885 4.865 4.853 4.843 4.810 4.778 4.753 4.700 4.610

4.850 4.878 4.905 4.885 4.850 4.860 4.823 4.810 4.730 4.700

4.918 4.920 4.960 4.968 4.915 [ 4.920] 4.905 4.875 4.802 4.780

4.970 4.970 5.028 5.020 4.980 5.000 4.970 4.958 4.898 4.878

5.048 5.070 5.103 5.113 5.095 5.100 5.070 5.080 5.005 4.973

5.183 5.140 5.230 5.210 5.220 5.218 5.200 5.157 5.175 5.093

5.265 5.363 5.348 5.355 5.345 5.350 5.358 5.363 5.290 5.293

5.453 5.518 5.555 5.585 5.580 5.573 5.550 5.555 5.508 5.435 (45,45) (375,45) (0,0) (9,0)

[Tannoo]

Hopefully it's just me screwing up somewhere.

[Roxy-3D]

Well... If this doesn't work on the Re-ARM, it is good to know. Because what ever is wrong needs to be fixed and the parsing code (like parser.seen()) is used everywhere. We better figure it out.

[Tannoo]

Yeah.

I know the LCD just sends G29 P6 X. Is that not correct?

[Roxy-3D]

There needs to be a C in front of the x.xx In UBL... pretty much any time you need a constant, it is specified with a C parameter.

[Tannoo]

Ok. It still didn't work in my tests of that. So, the C will be added to that line.

[Roxy-3D]

It is possible the parser stuff is broken on 32-bit. It may make sense to verify you can do a G29 P6 C1.00 on an AVR machine... And then do the same thing on the 32-bit machine.

[Tannoo]

void _lcd_ubl_adjust_height_cmd() {
      char UBL_LCD_GCODE[16];
      const int ind = ubl_height_amount < 0 ? 6 : 7;
      strcpy_P(UBL_LCD_GCODE, PSTR("G29 P6-"));
      sprintf_P(&UBL_LCD_GCODE[ind], PSTR(".%i"), abs(ubl_height_amount));
      enqueue_and_echo_command(UBL_LCD_GCODE);
    }

It works like that in AVR....hmmm Adding a C to the command won't break it?

[Roxy-3D]

It might be wise to print out UBL_LCD_GCODE when it gets enqueue'd. That shouldn't work without a C in front of the number. And especially... print out g29_constant to see what value it has.

[Tannoo]

Well, does this need to be across the board? G29 Tx? G29 Lx? G29 Sx?

[Tannoo]

Or does that only pertain to the P parameter?

[Tannoo]

Nevermind... I see it specified in the G29 P6 explination.

[Tannoo]

Okay... I got it to work.

[Tannoo]

It is now changed (in my branch) to this:

    /**
     * UBL Adjust Mesh Height Command
     */
    void _lcd_ubl_adjust_height_cmd() {
      char UBL_LCD_GCODE[16];
      const int ind = ubl_height_amount > 0 ? 9 : 10;
      strcpy_P(UBL_LCD_GCODE, PSTR("G29 P6 C -"));
      sprintf_P(&UBL_LCD_GCODE[ind], PSTR(".%i"), abs(ubl_height_amount));
      enqueue_and_echo_command(UBL_LCD_GCODE);
    }

[Roxy-3D]

Sorry... The internet comes and goes... I didn't see your messages until just now.

[Tannoo]

That's okay. I added the C in there and it works on the Re-ARM now.

[Roxy-3D]

Good! I'm fighting the Visual Studio / PlatformIO-IDE plugin. It would be great if we can figure out how to build the Re-ARM code there Because we can get a J-Link Debugger for $11 on eBay. (And I have one on order) But so far, I can't figure out how to make a Project and build it.

[Roxy-3D]

Hey! Quick Question: Did the CUSTOM_MENUS stuff ever get used by anybody? At https://github.com/MarlinFirmware/Marlin/issues/7590 Gordon is asking about making prints repeat. But looking through the code, I don't see any extra support for CUSTOM_MENUS or USERDESC? USERGCODE?. And the support functions _lcd_user_menu() and _lcd_user_gcode() are never referenced any where either?

I think what he wants to do is find a file name on an SD card, and print it. And then just press the encoder wheel and click it to print a 2nd (and 3rd and 4th) time. Can you let me know where the Custom Menu stuff is at? Because being able to repeat prints with just a Click would be very valuable to Gordon. And if we can't do it with Custom Menu's, I was going to write a dedicated function to do it for him.

[Tannoo]

CUSTOM_USER_MENUS in configuration_adv.h

[Roxy-3D]

Yes... But how is it used? At one point there was talk about bringing up the custom user menu's by doing a double click. That doesn't appear to be there. I don't see where the LCD code even tries to display the custom user menus.

[Tannoo]

If it is enabled, it will be in the main menu. I believe will be just before prepare.

[Tannoo]

I'm at work now and on the phone. I will look it up in a few.

[Tannoo]

PR #6895 for the user menu.

All changes for the user menu items are done in configuration_adv.h.

[Tannoo]

ultralcd.cpp is where the wrapper of CUSTOM_USER_MENUS allows the user menu to happen or not.

[Tannoo]

I think a dedicated function would probably be needed anyway and that can be called from the user menu. This new function should also be added to the SD menu as an option so that it doesn't get lost.

[Tannoo]

I say that to try and keep the custom user menu as clean as possible.

[Roxy-3D]

I think a dedicated function would probably be needed anyway and that can be called from the user menu. This new function should also be added to the SD menu as an option so that it doesn't get lost.

I think I have a really nice way to implement it with just a few lines of code. And then we add a #define to control whether the feature is turned on or not. If I can make it work, it should be hardly any code at all.

[Tannoo]

Shouldn't need another define.

Put a call to that function in the SD menu.

It can be also be called in the user menu for ease of access.

[Tannoo]

Then, it could be later called via a G or M code if it seems to be a useful function even with no LCD.

[Roxy-3D]

Shouldn't need another define.

No... I didn't mean another #define for the CUSTOM_MENUS. I meant in the SD Card section. Maybe something like

#define SD_RESELECT_LAST_PRINT   // if enabled, the LCD Panel menu will pre-select the last printed file
                                 // when the print finishes.  This facilitates easy re-prints of the same object

And then in Marlin_main.cpp where the SD card is read, at end of file it could do a lcd_goto_screen() that is slightly modified to start at the same place it was when a file was selected the last time.

[Tannoo]

I understand, but I don't think that define is needed.

[Tannoo]

Maybe a bool that can be toggled from the LCD.

[Roxy-3D]

Mostly... I was thinking that #define should be there just because some people will want the menu list to start at the first file. And others will want it to already be positioned in the 'right' place to print the same file again.

But I noticed something else that kind of makes me wonder about things. If the user drills down into folders on the SD Card to print... The next time the user goes to the SD Menu, it stays drilled down into that lower menu. That is fine if you want to print the same file again. But it kind of feels wrong if you are starting over....