Adjustments to M100

[thinkyhead]

Suggested changes for M100. Posting this to get a full Travis test, and for your consideration.

• Move mutually-exclusive M100 options into separate functions
• Apply initialization (and I parameter) before other parameters to M100
• Use ptr[i] in place of *(ptr + i)
• Indent comments at the level of the cascade
• Add a define TEST_BYTE which is set to 0xE5
• Rename how_many_E5s_are_here to count_test_bytes
• Use SERIAL_EOL / SERIAL_CHAR for single characters

[thinkyhead]

Aha! I just added Travis CI to my fork, so I can see test results before making a PR. Nice.

[Roxy-3D]

I have additional changes to make to M100. Now there is an additional debug function to dump the stack area. I don't have root cause on the memory corruption issue yet. But it is definitely tied to first access of the UBL object.

  void gcode_G29() {
free_memory_is_corrupted("Start of G29");
    if (ubl.eeprom_start < 0) {
      SERIAL_PROTOCOLLNPGM("?You need to enable your EEPROM and initialize it");
      SERIAL_PROTOCOLLNPGM("with M502, M500, M501 in that order.\n");
      return;
    }
free_memory_is_corrupted("Checkpoint 0000");

The first free_memory_is_corrupted() call says everything is fine. The second one fails. It is almost like the ubl object is not instantiated. But it is defined as a global in Marlin_main.cpp

#if ENABLED(AUTO_BED_LEVELING_UBL)
  #include "ubl.h"
  unified_bed_leveling ubl;
  #define UBL_MESH_VALID !( ( ubl.z_values[0][0] == ubl.z_values[0][1] && ubl.z_values[0][1] == ubl.z_values[0][2] \
                           && ubl.z_values[1][0] == ubl.z_values[1][1] && ubl.z_values[1][1] == ubl.z_values[1][2] \
                           && ubl.z_values[2][0] == ubl.z_values[2][1] && ubl.z_values[2][1] == ubl.z_values[2][2] \
                           && ubl.z_values[0][0] == 0 && ubl.z_values[1][0] == 0 && ubl.z_values[2][0] == 0 )  \
                           || isnan(ubl.z_values[0][0]))
#endif

so that idea makes no sense. However... Very clearly... the stack is getting corrupted by accessing ubl.eeprom_start. I can see the stack pointer get radically altered between before the 'if' and after the 'if'.

@thinkyhead Can you take a look at things and see if something obvious jumps out for you?

PS. Go ahead and merge these changes. I don't need things stable any more. Thanks for waiting!

[thinkyhead]

Go ahead and merge these changes.

Will do.

As for the memory corruption issue, I'm still at a loss. I wonder if it's worth taking a look at the assembly code produced by the compiler at that point? Not that the compiler is choking, but maybe it will reveal something about the way it's accessing the ubl object.

[Roxy-3D]

As for the memory corruption issue, I'm still at a loss. I wonder if it's worth taking a look at the assembly code produced by the compiler at that point? Not that the compiler is choking, but maybe it will reveal something about the way it's accessing the ubl object.

YES!!!! I haven't been able to do to that. It goes from good to bad in that very small section of code. And... That is the very first time the ubl object is accessed. So right now, I'm leaning towards a ubl object instantiation issue. But I don't know what I'm fighting. The assembly language would help a lot!

[thinkyhead]

a ubl object instantiation issue

Thing is, as a static class it's totally instantiated (except for calling the constructor) at compile-time. So there shouldn't be anything special going on, unless the constructor is doing something bad.

[Roxy-3D]

a ubl object instantiation issue

Thing is, as a static class it's totally instantiated (except for calling the constructor) at compile-time. So there shouldn't be anything special going on, unless the constructor is doing something bad.\

Even if it wasn't a static class... If it is a global object, doesn't the C Run Time library 'instantiate' it? As for constructors... There is isn't much going on. I'll know more tomorrow if the UBL object is the problem. But right now... there isn't much else in those 4 or 5 lines of code that could be causing a problem.

The constructor doesn't do much:

  unified_bed_leveling::unified_bed_leveling() {
    for (uint8_t i = 0; i < COUNT(mesh_index_to_xpos); i++)
      mesh_index_to_xpos[i] = UBL_MESH_MIN_X + i * (MESH_X_DIST);
    for (uint8_t i = 0; i < COUNT(mesh_index_to_ypos); i++)
      mesh_index_to_ypos[i] = UBL_MESH_MIN_Y + i * (MESH_Y_DIST);
    reset();
  }

I'll be double checking everything tomorrow... The fact the Stack Pointer ends up in the wrong place is probably a valuable clue. I wish I could get an .iii file from the compiler with the source code mixed in with the assembly code. I might be able to see how the Stack Pointer is getting confused.

[thinkyhead]

doesn't the C Run Time library 'instantiate' it?

Well I'm just pointing out that a "first access" of a data member in this case shouldn't cause anything else to occur (such as invoking the initializer behind the scenes). So if simply accessing ubl.eeprom_start causes corruption, it implies the object is already corrupted in some manner, and not necessarily at this point.

Next, try doing two free_memory_is_corrupted calls in a row with nothing in-between, just to make sure that the first call to free_memory_is_corrupted isn't the very thing causing the corruption.

[Roxy-3D]

Next, try doing two free_memory_is_corrupted calls in a row with nothing in-between, just to make sure that the first call to free_memory_is_corrupted isn't the very thing causing the corruption.

Yes. This has been done. (and in fact, it was done with 5 calls in a row, before and after.) The function behaves properly.

Well I'm just pointing out that a "first access" of a data member in this case shouldn't cause anything else to occur (such as invoking the initializer behind the scenes). So if simply accessing ubl.eeprom_start causes corruption, it implies the object is already corrupted in some manner, and not necessarily at this point.

Yes. Perhaps. A hardware debugger would be very helpful right now. Something else I'm wondering about is the difference between a static object and a normal object. The first is kind of like a normal global, and the other is accessed via its 'this->' pointer, right? I'm wondering if there is a mis-match in how the object is created and how it is being referenced between the different files.

[thinkyhead]

The first is kind of like a normal global, and the other is accessed via its 'this->' pointer, right?

Yes. The this value only applies within non-static methods of a class, since only those methods receive this as the hidden first argument. (The class instance itself has no "this" data member, so it will sometimes be simulated for singleton classes with a self or instance data member — a pointer to the single instance.)

When calling a static member function you can use the bare name of the function from within other methods of the same class. All other code must prefix with either the name of some instance (which is a reference, not a pointer) or the name of the class. So all the following calls to do_something are equivalent:

void some_function() {
  ubl.do_something(); // instance reference
  unified_bed_leveling::do_something(); // class namespace
}

void unified_bed_leveling::do_more_things() { do_something(); } // same namespace

No matter how many instances of an entirely static class are created, there will only be 1 copy of the data and the functions, and all the references will produce identical code.

unified_bed_leveling ubl1, ubl2;

ubl1.do_something();
ubl2.do_something(); // same 'object'