UBL Menu Sytem

[Tannoo]

@Roxy-3D, could you look this over and tell me what you think?

https://github.com/Tannoo/Marlin/tree/UBL_Menu

It is current with RCBugFix as of 4/20/17.

[Tannoo]

A command like G29 J4 would be useful on your menu.

 *    Mesh Leveling
 *      Mesh Leveling <-- 3-Point
 *      Grid Mesh Leveling
 *        Side points:  <-- Custom Points
 *        Level Mesh  <-- Run with Custom Points

    /**
     * UBL Mesh Leveling submenu
     */
    void _lcd_ubl_mesh_leveling() {
      START_MENU();
      MENU_BACK(MSG_UBL_LEVEL_BED);
      MENU_ITEM(gcode, MSG_UBL_MESH_LEVELING, PSTR("G29 T"));  <-- 3-Point tilting
      STATIC_ITEM(MSG_UBL_GRID_MESH_LEVELING, true);
      MENU_ITEM_EDIT(int3, MSG_UBL_SIDE_POINTS, &SIDE_POINTS, 2, 5);  <-- Set 2 - 5 points
      MENU_ITEM(function, MSG_UBL_MESH_LEVEL, _lcd_ubl_mesh_level_cmd);  <-- Run with Custom points.
      END_MENU();
    }

    /**
     * UBL Mesh Leveling Command
     */
    void _lcd_ubl_mesh_level_cmd() {
      sprintf_P(SIDE_POINTS_GCODE, PSTR("G29 J%i"), SIDE_POINTS);  <-- Here's your Jx command.
      enqueue_and_echo_command(SIDE_POINTS_GCODE);
    }

I'll rename MSG_UBL_MESH_LEVELING to MSG_UBL_3POINT_MESH_LEVELING.

[Tannoo]

tilt the mesh

Mesh leveling?

[Tannoo]

Or would you like me to rename that to Mesh Tilting?

[Tannoo]

Do you think that a range of 2 to 5 is sufficient for the mesh tilting?

[Roxy-3D]

Probably a Range of 3 to 6 is more than sufficient for Mesh Tilting. Rather than have a PLA Mesh command and an ABS Mesh Command... It would make sense to allow the user to Specify which mesh slot to save (or load) the mesh. I use the same mesh for different materials. But I do have multiple mesh's defined and saved based on which piece of glass I'm using.

[Tannoo]

J # Grid * Perform a Grid Based Leveling of the current Mesh using a grid with n points on a side. I got Mesh Leveling from that description.

[Roxy-3D]

Yes... This is actually a combination of the Mesh Leveling idea with the planar Grid Leveling. The mesh might be perfectly tuned for a piece of glass. But if a small piece of plastic ends up under the glass... That mesh will be slightly tilted.

[Tannoo]

Rather than have a PLA Mesh command and an ABS Mesh Command... It would just make sense to allow the user to Specify which mesh slot to save (or load) the mesh.

I can remove the PLA and ABS options. Leave the Custom? to set desired temps? Or remove that also?

Selecting a storage slot and loading or saving to that slot is already there. Was there something else you had in mind?

    /**
     * UBL Mesh Storage submenu
     */
    void _lcd_ubl_storage_mesh() {
      START_MENU();
      MENU_BACK(MSG_UBL_LEVEL_BED);
      MENU_ITEM_EDIT(int3, MSG_UBL_STORAGE_SLOT, &UBL_STORAGE_SLOT, 0, 9);  <--Slot Select
      MENU_ITEM(function, MSG_UBL_LOAD_MESH, _lcd_ubl_load_mesh); <-- go load it.
      MENU_ITEM(function, MSG_UBL_SAVE_MESH, _lcd_ubl_save_mesh);  <-- go save it.
      END_MENU();
    }

    /**
     * UBL Load Mesh Command
     */
    void _lcd_ubl_load_mesh() {
      sprintf_P(UBL_STORAGE_GCODE, PSTR("G29 N L%i"), UBL_STORAGE_SLOT);  <-- Load selected slot
      enqueue_and_echo_command(UBL_STORAGE_GCODE);
    }

    /**
     * UBL Save Mesh Command
     */
    void _lcd_ubl_save_mesh() {
      sprintf_P(UBL_STORAGE_GCODE, PSTR("G29 N S%i"), UBL_STORAGE_SLOT);  <-- Save selected slot
      enqueue_and_echo_command(UBL_STORAGE_GCODE);
    }

[Tannoo]

This is actually a combination of the Mesh Leveling idea with the planar Grid Leveling. The mesh might be perfectly tuned for a piece of glass. But if a small piece of plastic ends up under the glass... That mesh will be slightly tilted.

I understand. Did you want it to stay as Mesh Leveling or title it to something else?

[Roxy-3D]

I can remove the PLA and ABS options. Leave the Custom? to set desired temps? Or remove that also?

I understand now why you think that is important. I believe it makes sense to let the user probe the bed (or for that matter do a G26) at different temperatures. Maybe we could have a common routine that gets the bed and nozzle temperature, and then uses that to enque the G29 P1 or the G26 commands?

But also... For the bed and nozzle heating... There are already #defines for PLA and ABS. So using those would be just fine. The more I think about it... Having the same command with PLA or ABS on the title is just fine. People will instantly know what is what.

Selecting a storage slot and loading or saving to that slot is already there. Was there something else you had in mind?

I think what you are doing is good! People are going like using your LCD Menu System and they are not going to have trouble getting it to do the right thing for them.

Did you want it to stay as Mesh Leveling or title it to something else?

For G29 J, Probably Mesh Tilting is the correct thing to call it.

[Tannoo]

I value your insight. Just trying to understand what you would like to see.

The generate pla mesh is for preheating before probing to pla temps. -- yes, I should get those temps from the system defines instead of the set temps I placed. Ditto for generate ABS mesh.

If I were to consolidate those to just one generate mesh, what temps to use? PLA or ABS.

I have found that when doing the UBL probing @ ABS temps with PLA in the hotend, it is way to hot for too long due to no extrusion taking place. This could result in clogging.

[Roxy-3D]

If I were to consolidate those to just one generate mesh, what temps to use? PLA or ABS.

The more I think about it... It is more intuitive for the user to have both specified and available.

I have found that when doing the UBL probing @ ABS temps with PLA in the hotend, it is way to hot for too long due to no extrusion taking place. This could result in clogging.

Yeah... It is more accurate to do your probing at full temperature. But in that case... Starting with a PLA grid and doing a G26 at ABS temperatures (using ABS) would work fine. You would then edit the mesh to correct some locations. And maybe have to do another iteration to get things perfect. But you don't have to do the probing with the nozzle at full temperature to end up with a perfect mesh.

But there isn't going to be any way to communicate that on the LCD Panel. Probably this type of 'Helpful Hint' would go into the Quick Start guide that Bob-the-Kuhn is doing.

[Tannoo]

Okay.

[Tannoo]

I think I have conserved some memory by using the same variable for gcodes. I will be testing this to make sure all is still kosher.

[Tannoo]

This doesn't work like it used to. It now runs the first command and stops.

      sprintf_P(CUSTOM_MESH_GCODE, PSTR("G28\nM190 S%i\nM109 S%i\nG29 P1\nM104 S0\nM140 S0"), PREHEAT_1_TEMP_BED, PREHEAT_1_TEMP_HOTEND);
      enqueue_and_echo_command(CUSTOM_MESH_GCODE);

I have to split it up like this to work:

      enqueue_and_echo_command("G28");
      sprintf_P(CUSTOM_MESH_GCODE, PSTR("M190 S%i"), PREHEAT_1_TEMP_BED);
      enqueue_and_echo_command(CUSTOM_MESH_GCODE);
      sprintf_P(CUSTOM_MESH_GCODE, PSTR("M109 S%i"), PREHEAT_1_TEMP_HOTEND);
      enqueue_and_echo_command(CUSTOM_MESH_GCODE);
      enqueue_and_echo_command("G29 P1");
      enqueue_and_echo_command("M104 S0");
      enqueue_and_echo_command("M140 S0");

I will try an older copy of Marlin to try and see what changed. (Because, I know a LOT has changed in a few days now.)

[Roxy-3D]

Yeah... It definitely claims that is possible! Are you calling drain_injected_commands_P() ?
Probably you should start an issue and point to these comments.

/**
 * Inject the next "immediate" command, when possible, onto the front of the queue.
 * Return true if any immediate commands remain to inject.
 */
static bool drain_injected_commands_P() {
  if (injected_commands_P != NULL) {
    size_t i = 0;
    char c, cmd[30];
    strncpy_P(cmd, injected_commands_P, sizeof(cmd) - 1);
    cmd[sizeof(cmd) - 1] = '\0';
    while ((c = cmd[i]) && c != '\n') i++; // find the end of this gcode command
    cmd[i] = '\0';
    if (enqueue_and_echo_command(cmd))     // success?
      injected_commands_P = c ? injected_commands_P + i + 1 : NULL; // next command or done
  }
  return (injected_commands_P != NULL);    // return whether any more remain
}

/**
 * Record one or many commands to run from program memory.
 * Aborts the current queue, if any.
 * Note: drain_injected_commands_P() must be called repeatedly to drain the commands afterwards
 */
void enqueue_and_echo_commands_P(const char* pgcode) {
  injected_commands_P = pgcode;
  drain_injected_commands_P(); // first command executed asap (when possible)
}

[Tannoo]

The Serial monitor will show:

enqeueing "G28
M190 S50
M109 S195
G29 P1
M104 S0
M140 S0"

But, it runs G28 and stops.

[Roxy-3D]

But, it runs G28 and stops.

Are you calling drain_injected_commands_P() ?

[Tannoo]

sprintf_P(CUSTOM_MESH_GCODE, PSTR("M190 S%i\nM109 S%i\nG29 P1\nM104 S0\nM140 S0"), PREHEAT_2_TEMP_BED, PREHEAT_2_TEMP_HOTEND);
enqueue_and_echo_command(CUSTOM_MESH_GCODE);

[Roxy-3D]

Yeah... But I think it wants you to follow up with something like:

while (drain_injected_commands_P())
  idle();

[Tannoo]

is sprintf_P a drain_injected_commands_P?

[Tannoo]

Or, is that what the _P is for?

[Roxy-3D]

No. sprintf_P() almost certainly is a 'Program Memory' version of sprintf(). Probably (without looking it up), it is taking the format string out of 'Program Memory'. It has very little to do with things like drain_injected_commands_P();

And I just noticed another issue... drain_injected_commands_P(); is trying to take the commands it enque()'s out of program memory. That is what the _P means. It uses strncpy_P(cmd, injected_commands_P, sizeof(cmd) - 1); which goes into program memory to get the string. But your sprintf_P() put the results of the string formatting operation into RAM memory. Something needs to change. Probably there is an enque() command that does not have a _P on the end of it.

[Tannoo]

ok. I have found that this: MENU_ITEM(gcode, MSG_UBL_BUILD_PLA_MESH, PSTR("G28\nM190 S50\nM109 S195\nG29 P1\nM104 S0\nM140 S0")); works just fine with the results like:

enqueueing "G28"
enqueueing "M190 S50"
enqueueing "M109 S195"
enqueueing "G29 P1"
enqueueing "M104 S0"
enqueueing "M109 S0"

I have found that I can't get the sprint_P to do that.

[Roxy-3D]

The PSTR("---") accesses a character string that is located in Program Memory. sprintf() and sprintf_P() are going to write their formatted output into a character string located in RAM memory...

[Tannoo]

enqueue_and_echo_command(CUSTOM_MESH_GCODE);

So, is this my problem? Not using the enqueue_and_echo_commands_P?

[Tannoo]

I'm not thinking so.. because the new results aren't promising:

echo:enqueueing " "
echo:Unknown command: ""

[Roxy-3D]

The root of the problem is the AVR is a "Harvard Architecture". The program data space overlaps the RAM memory data space. You have to make sure the pointer and functions you are using are accessing the right data space.

If something has a _P on it... That means it is going to try to get a character string out of program memory. And program memory can only be altered by loading Arduino, recompiling the firmware, and uploading it.

I know it is confusing... And that is what that PSTR() thing is. That is a macro that forces what ever string you put inside it to go into program memory instead of eating up RAM memory.

[Tannoo]

Ok. I am trying to get something work now. I'll split it up for now.

[Tannoo]

Maybe sprintf_P just can't handle that much.

[Roxy-3D]

Maybe sprintf_P just can't handle that much.

No... I think what is happening is sprintf_P() is taking its format string out of program memory. But the output from that function is ending up in RAM memory. (You can't change program memory from within the program. The only place it can put its results is into RAM memory.)

If you are going to enque() something that is in RAM, you can't be using an enque_P() type of command.

[Tannoo]

M'kay.

[Tannoo]

I'm using this instead and it works like it's supposed to.

MENU_ITEM(gcode, MSG_UBL_BUILD_PLA_MESH, PSTR("G28\nM190 S" STRINGIFY(PREHEAT_1_TEMP_BED) 
  "\nM109 S" STRINGIFY(PREHEAT_1_TEMP_HOTEND) "\nG29 P1\nM104 S0\nM140 S0"));

[Roxy-3D]

Great! The only problem with that is you can't change it once the firmware is loaded... The user is stuck with what ever it was when the firmware is built. But still... That is going to be just fine!!! This is going to be super helpful to people!!!

[Tannoo]

That was for the PLA mesh build.

This is for custom temps:

    void _lcd_ubl_custom_mesh() {
      START_MENU();
      MENU_BACK(MSG_UBL_LEVEL_BED);
      MENU_ITEM_EDIT(int3, MSG_UBL_CUSTOM_HOTEND_TEMP, &CUSTOM_HOTEND_TEMP, EXTRUDE_MINTEMP, HEATER_0_MAXTEMP);
      MENU_ITEM_EDIT(int3, MSG_UBL_CUSTOM_BED_TEMP, &CUSTOM_BED_TEMP, BED_MINTEMP, BED_MAXTEMP);
      MENU_ITEM(gcode, MSG_UBL_BUILD_CUSTOM_MESH, PSTR("G28\nM190 S" STRINGIFY(CUSTOM_BED_TEMP)
        "\nM109 S" STRINGIFY(CUSTOM_HOTEND_TEMP) "\nG29 P1\nM104 S0\nM140 S0"));
      END_MENU();
    }

[Tannoo]

I have a question though..

Is HEATER_0_MAXTEMP suffice for the active extruder? Or is leveling only active for extruder 0?

[Roxy-3D]

Right now... The G26 stuff assumes it will use extruder 0. Of course.. eventually... it will support what ever extruder is on the machine.

Is HEATER_0_MAXTEMP suffice for the active extruder?

I don't think you can use this... If you set the temperature to this number... even a .1 degree over shoot will cause Kill() to be called. And HEATER_0_MAXTEMP is going to be a bigger number than what ever actually gets used. It is a hard limit to shut down the printer if it ever happens.

[Tannoo]

Ok. What should I use instead?

[Roxy-3D]

In Configuration.h there are now values for:

// Preheat Constants
#define PREHEAT_1_TEMP_HOTEND 200
#define PREHEAT_1_TEMP_BED     70
#define PREHEAT_1_FAN_SPEED     0 // Value from 0 to 255

#define PREHEAT_2_TEMP_HOTEND 240
#define PREHEAT_2_TEMP_BED    110
#define PREHEAT_2_FAN_SPEED     0 // Value from 0 to 255

Probably these are the values that should be in the 'STRINGIFY()' thing....

They are not called out as PLA and ABS. But I think things are sorting out such that 1 will be PLA and 2 will be ABS.

[Tannoo]

Yup. I got those.

For custom temps, one may want a higher temp than those.

[Roxy-3D]

OK... But here is the thing... If you are going to let the user dial in a temperature... You have to enque() the command without an _P functions in play. I would have to look at the enque() stuff to know for sure. My guess is there is one that will work with RAM based command strings. But if there isn't... It isn't going to be hard to make one that does the same thing except it takes its string from RAM. After all...

static bool drain_injected_commands_P() {
  if (injected_commands_P != NULL) {
    size_t i = 0;
    char c, cmd[30];
    strncpy_P(cmd, injected_commands_P, sizeof(cmd) - 1);
    cmd[sizeof(cmd) - 1] = '\0';
    while ((c = cmd[i]) && c != '\n') i++; // find the end of this gcode command
    cmd[i] = '\0';
    if (enqueue_and_echo_command(cmd))     // success?
      injected_commands_P = c ? injected_commands_P + i + 1 : NULL; // next command or done
  }
  return (injected_commands_P != NULL);    // return whether any more remain
}

That is what the strncpy_P(cmd, ...) is doing. It is copying the command that it has been told to enque() out of program memory and moving it to RAM. So you would just skip that step because the stuff you want to do is already in RAM. Does that make sense?

[Tannoo]

Yes.

[Tannoo]

Well, like you stated, the custom data is not working.

MENU_ITEM(gcode, MSG_UBL_BUILD_CUSTOM_MESH, PSTR("G28\nM190 S" STRINGIFY(CUSTOM_BED_TEMP)
        "\nM109 S" STRINGIFY(CUSTOM_HOTEND_TEMP) "\nG29 P1\nM104 S0\nM140 S0"));

[Tannoo]

Maybe we have a little postage stamp size icon with a dot on it representing which number (or group of numbers) is being displayed. And the dot moves around based on how the encoder wheel is turned?

I like that idea! BUT, I am not the dude for that job. :(

[Roxy-3D]

Maybe we have a little postage stamp size icon with a dot on it representing which number (or group of numbers) is being displayed. And the dot moves around based on how the encoder wheel is turned?

I like that idea! BUT, I am not the dude for that job. :(

Not to worry... Once people start using the LCD Panel as the interface... Somebody will jump in and say "We need the topology map on the LCD too."

[Tannoo]

Did you take out this description on purpose? I use it in the menu options because not all options require homing.

   *   N    No Home     G29 normally insists that a G28 has been performed. You can over rule this with an
   *                    N option. In general, you should not do this. This can only be done safely with
   *                    commands that do not move the nozzle.

[Tannoo]

Once people start using the LCD Panel as the interface...

The UBL menu will evolve. I also believe that as UBL becomes more automated, this menu will also shrink and may also be nothing more than an option to start it as ABL is currently.

[Roxy-3D]

Yes. N is a special letter in GCode to say what line was transmitted. It causes problems with Repetier Host. And, pretty much, users shouldn't be using it. It was only in there for me (and now you). So... I took it out of the description just so there would be less controversy.

[Tannoo]

Nevermind

[Tannoo]

PR has now been submitted. #6566