Marlin: UBL not working. Please fix.

[Roxy-3D]

Everything has changed. The function names aren't even recognizable to me any more.

Nothing works. It can't even load a mesh any more.

It can't display a mesh.

It can't probe and acquire a mesh.

Almost for sure if those are fixed it won't do z-height correction properly.

I didn't even bother looking at g26 other than to see the code littered with those f$%#&'ing ternary operators I hate. I'm the one supporting the code. Those make it harder for me.

---

It isn't fair to be making radical changes and not even test things to see if basic operation still exists. It is not fair to the users. And it certainly is not fair to me.

What was even the purpose of me spending time to load your updated version and test it for proper operation? That file set can't even be compared to what was merged.

Let's not revert anything. Instead... Let's see how many weeks it takes you to get things running so you can do a print.

[ghost]

It isn't fair to be making radical changes and not even test things to see if basic operation still exists. It is not fair to the users. And it certainly is not fair to me.

Yeah, a frustration of mine for a while now. I have been seeing PR's merged before little (if any) testing.
I just deemed it "Bleeding edge" code.

[alexborro]

That happens when the project do not have a build coordinator. That was one
of the reasons I gave up the Marlin mainline.

[thinkyhead]

Sorry, folks. I should have made sure the code was compliant before pulling it into the branch. In my experience when something is _totally broken_ it's usually due to something easier to find than when something is only acting weird.

I did some testing of my patches, and it was working for me, but perhaps a last-minute patch before bed broke something. I will of course see if I can track down the issue, keep on testing, and get it back to working order.

So anyway, let me make a branch for testing of updated UBL code, revert RCBugFix to the last working commit of UBL, and we'll get it working within the day, I'm sure. This isn't rocket science, just basic procedural-style C++. The usual debugging methodologies will get us where we need to go.

[thinkyhead]

I gave up the Marlin mainline.

Well, thanks for your past contributions anyway. It's hard to find good help.

[alexborro]

Thanks Scott.
I do admire all your efforts on Marlin.
I keep an eye on updates - often I get good ideas.

If I could give just one advice, go for a build coordinator. That will save
you guys A LOT of headache, a lot of reworks, bugfixes, etc..

Cherrs.
Alex.

[thinkyhead]

go for a build coordinator

It's a good idea. We don't do "builds", per se, because every system is different. But we should definitely have someone on hand who can certify the last well-tested version and make that a public-facing numbered RC (the public being our test-base), while we keep our cutting-edge stuff in a more cutting-edge location…. such as a bug-fix branch.

[Roxy-3D]

For starters.... Rule #1 USER DATA IS SACRED

Maybe it makes sense to change this data structure. But to save a couple bytes of EEPROM (That don't even matter) I don't want to lose all my mesh data. On my gMax I have 3x 15x15 grids that are perfectly tuned for the different pieces of glass I use. On my Folger Tech i3-2020 I have two different pieces of glass with 10x10 grids (that are perfectly tuned). And I just brought up UBL on my old Frankenstein i3. That is also a 10x10 grid but not yet perfectly tuned. I don't want to lose my work (and data) just because somebody wants to squish a couple of bytes out of a data structure.

And if you look at the data structure... I even envisioned it would change.

      unsigned char padding[64 - TOTAL_STRUCT_SIZE];

I added padding so it could grow. If you were going to squish out some unneeded space... We could have a few bytes of 'To_Be_Allocated_for_What_Ever' but leave my data intact. This is item #1 that needs to be repaired. Without this... I can't load my mesh data to check out the rest of the changes as they arrive.

Seriously Scott... You know I appreciate your dedication and expertise. Probably the reason I'm reacting so badly is I've worked very hard to make sure the user has a 'Good Experience' when they bring up UBL. I really do believe it is the "Cat's Meow" of Bed Leveling. Making arbitrary changes that break the whole system makes it look like I did a sloppy job.

    typedef struct {
      bool active = false;
      float z_offset = 0.0;
     **int8_t** eeprom_storage_slot = -1,
             n_x = UBL_MESH_NUM_X_POINTS,
             n_y = UBL_MESH_NUM_Y_POINTS;

      float mesh_x_min = UBL_MESH_MIN_X,
            mesh_y_min = UBL_MESH_MIN_Y,
            mesh_x_max = UBL_MESH_MAX_X,
            mesh_y_max = UBL_MESH_MAX_Y,
            mesh_x_dist = MESH_X_DIST,
            mesh_y_dist = MESH_Y_DIST;

      #if ENABLED(ENABLE_LEVELING_FADE_HEIGHT)
        float g29_correction_fade_height = 10.0,
              g29_fade_height_multiplier = 1.0 / 10.0; // It's cheaper to do a floating point multiply than divide,
                                                       // so keep this value and its reciprocal.
      #else
        const float g29_correction_fade_height = 10.0,
                    g29_fade_height_multiplier = 1.0 / 10.0;
      #endif

      // If you change this struct, adjust TOTAL_STRUCT_SIZE

      #define TOTAL_STRUCT_SIZE 40 // Total size of the above fields

      // padding provides space to add state variables without
      // changing the location of data structures in the EEPROM.
      // This is for compatibility with future versions to keep
      // users from having to regenerate their mesh data.
      unsigned char padding[64 - TOTAL_STRUCT_SIZE];
    } ubl_state;

[ghost]

But we should definitely have someone on hand who can certify the last well-tested version and make that a public-facing numbered RC

This would be good, especially if there were frequent RC releases.

There should be a timeline to release the next RC. That would also reduce the amount of code to "stabilize".

[ghost]

This would also reduce the amount of people running stale releases.
It does seem that the best support responce on most issues with the last "stable" release is to run RCBugFix.

[alexborro]

go for a build coordinator

It's a good idea. We don't do "builds", per se, because every system is different.

I'm aware of that. But the Build Coordinator job is more than compiling. I have done that for years in huge projects. The BC is responsible for accepting and filter any changes entering the code. He is responsible for bugfixes version, release versions and so one.

Today you have a living Marlin. There is no release version, no quality control at all. There is bugfixes, new features, new ideas, etc, all mixed together in same branch. That is the recipe for tragedies like this.

The only version you have today is the RC, that has had a long life. A release candidate version must not have new features added. Just controlled bugfixes.

Today Marlin is a complete mess in version control - please don't take any offense for that, it is just a feedback for improvement.

At least once a week I see emails complaining about bug in the UBL. And it keeps entering new code and "features" into it.

Again, don't take me wrong in these comments, it is just an idea to improve your job and minimize efforts.

Cheers.
Alex.

[thinkyhead]

All good points. But, you know, at the moment I'm going through dozens of issues that no one has answered, dating back to December. So we really are short on people who want to do the grunt work and not just their pet projects. I'd love to have more experienced, competent, and mature help.

[Roxy-3D]

At least once a week I see emails complaining about bug in the UBL. And it keeps entering new code and "features" into it.

The last bug I was working on before everything was broken was this one. https://github.com/MarlinFirmware/Marlin/issues/6175

All of a sudden the Mesh Topology Map of UBL quit working. Here is why... In the Release Candidate, in very well tested code, an arbitrary change was made. These lines of code changed from this

    int8_t i, j;
...
    for (j = UBL_MESH_NUM_Y_POINTS - 1; j >= 0; j--) {
      for (i = 0; i < UBL_MESH_NUM_X_POINTS; i++) {

to

    for (uint8_t j = UBL_MESH_NUM_Y_POINTS - 1; j >= 0; j--) {
      for (uint8_t i = 0; i < UBL_MESH_NUM_X_POINTS; i++) {

This was well tested code. Those loop variables live on the stack. What benefit did we get by changing them to uint8_t ? I don't see a benefit. But I do know a user was inconvenienced because of the untested change that was made. And more emails went out to people like Alexborro telling him UBL doesn't work and is plagued with bugs.

I would like to see us use this as a positive experience that emphasizes the need to only do development in development branches. I think everybody wants https://github.com/MarlinFirmware/Marlin/issues/6188 to happen.

How about we all agree we only do Bug Fixes to RCBugFix? And maybe in a month, we will have something stable enough to be promoted?

[p3p]

Making arbitrary changes that break the whole system makes it look like I did a sloppy job

@Roxy-3D No one thinks less of you, or your ability, for a few bumps in the merge process of a large feature pull request into the main unstable development branch. Of course it is understandably frustrating when the issues are caused by optimisations and cleanup during the merge but that's inevitable in any large project.

I think a large part of the problem is the unclear nature of the branch naming RC, RCBugFix should both have been under feature freeze, there should be a main development branch for well, development, and those of us with the technical ability and desire to be on the bleeding edge.

Rule #1 USER DATA IS SACRED

Unfortunately that rule only counts in stable releases, and as it is now, the unstable development branch and the branch with the bug fixes that I see recommended time and again in issues to the general user base, is the same branch, RCBugFix

[ghost]

RC, RCBugFix should both have been under feature freeze

I agree. Bring back a DEV branch for new features.

[Roxy-3D]

RC, RCBugFix should both have been under feature freeze

I agree. Bring back a DEV branch for new features.

I would like to see us use this as a positive experience that emphasizes the need to only do development in development branches. I think everybody wants #6188 to happen. How about we all agree we only do Bug Fixes to RCBugFix? And maybe in a month, we will have something stable enough to be promoted?

Yes. But let's also get a rock solid stable release tagged.

[lrpirlet]

@Roxy-3D @thinkyhead
I wanted to go with the latest RCBugFix yesterday when I saw this thread... Is UBL functional again? I could not find any confirmation that it has been patched back to "normal", and the message in the original title "_do not use it_" is frightening...

Thanks in advance for any answer... Thanks again for the huge work done on Marlin firmware...

[Roxy-3D]

I wanted to go with the latest RCBugFix yesterday when I saw this thread... Is UBL functional again? I could not find any confirmation that it has been patched back to "normal", and the message in the original title "do not use it" is frightening...

I am running it on my two 'nice' printers. (I am careful about what firmware I load on my 'nice' printers!) I am printing parts with it. But you can not use your mesh from EEPROM. What I did was export my mesh data via a G29 S-1 and then flash the firmware to the latest. Then I loaded the exported data (as a .GCode file) and 'printed' that data to get my mesh into the new firmware. I saved the mesh and I can print. BUT... You can not load your mesh from EEPROM and use it. It will be corrupted because the data structures changed.

[lrpirlet]

Thanks @Roxy-3D...

I thought that when a change was introduced to the EEPROM, the checksum
would prevent it to make it's way to the in memory parameters set...

I have a functional mesh in EEprom for the moment and not to be able to
use the EEPROM data is a serious "no go"... I'll wait till that is fixed
to proceed... I feel like reloading the mesh via g-code each time is a
nice workaround, yet still a workaround (that change the way to print,
implying documentation, and education to read that doc...)

Anyway, thanks for this very nice UBL functionality...

[Roxy-3D]

I have a functional mesh in EEprom for the moment and not to be able to use the EEPROM data is a serious "no go"... I'll wait till that is fixed to proceed... I feel like reloading the mesh via g-code each time is a nice workaround

I understand your point, but that isn't how it is... This is a "Do it once and everything is just like it used to be." Just follow the directions in the first post of https://github.com/MarlinFirmware/Marlin/issues/6231 when you update the firmware.