Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move UBL mesh_index_to_x/y from sram to progmem #6432

Merged
merged 1 commit into from Apr 22, 2017

Conversation

bgort
Copy link
Contributor

@bgort bgort commented Apr 22, 2017

Tested and working well here. Saves 120 bytes of SRAM (in the case of a 15x15 mesh).

Discussed here: #6424

fix maximum mesh_index_ array size at 16 (15+1);
@Roxy-3D Roxy-3D merged commit f1a4758 into MarlinFirmware:RCBugFix Apr 22, 2017
@Roxy-3D
Copy link
Member

Roxy-3D commented Apr 22, 2017

 constexpr float unified_bed_leveling::mesh_index_to_xpos[16],
                 unified_bed_leveling::mesh_index_to_ypos[16];

I thought this would have to have PROGMEM in it? But I guess the compiler likes what you have or Travis would have complained.

@bgort
Copy link
Contributor Author

bgort commented Apr 22, 2017

Yeah -- it works and there are no errors or warnings. The actual definition where PROGMEM matters is in the header.

@thinkyhead
Copy link
Member

thinkyhead commented Apr 25, 2017

So we've decided not to implement variable grid size for UBL…?

I was hoping to extend leveling to allow the number of grid points and the probe region to be variable with other forms of leveling (as they are currently with AUTO_BED_LEVELING_LINEAR). I actually need this capability for my client, so I'm adding it to AUTO_BED_LEVELING_BILINEAR.

But, it would be good to be able to make it fixed and stored in PROGMEM if there's a need for extra SRAM.

@bgort
Copy link
Contributor Author

bgort commented Apr 25, 2017

What do you mean by variable, specifically? Without recompiling?

@thinkyhead
Copy link
Member

thinkyhead commented Apr 25, 2017

It means that you can specify a different region of the bed or a different number of probe points when running G29… without recompiling.

@bgort
Copy link
Contributor Author

bgort commented Apr 25, 2017

Ah, yeah. Not sure what it'd take to make UBL variable in that way, but obviously the recent progmem-related changes don't help.

@thinkyhead
Copy link
Member

Well, we've got one week to patch up stuff for release, so we'll revisit this for 1.1.1 / 1.2.

@bgort
Copy link
Contributor Author

bgort commented Apr 25, 2017

Makes sense to me.

Out of curiosity - and if you don't mind sharing - what's the use case requiring variable leveling?

@Roxy-3D
Copy link
Member

Roxy-3D commented Apr 25, 2017

Well, we've got one week to patch up stuff for release,

@thinkyhead @bgort @oldmcg
Check out: #6241 (comment)

Seriously.... We can have UBL working on Deltas if we wait an extra week. If a few people with Deltas pitch in... It is a no-brainer. Right now... I'm OK with pushing the release out to the first week of April. The Marlin user base gets a big bonus if we do that. All of the Delta users will have a 'Stable Release' with UBL in it working for them. And I'll have the Grid Based leveling in it along with the 'Smart Fill' command.

So we've decided not to implement variable grid size for UBL…?

Actually... That is almost done. It is a fall out of @Bob-the-Kuhn 's grid code he gave me to start with. You can specify the probe matrix except it is with a J number and not a P number. All of the P numbers are used for the various 'Phases' you go through to bring up a UBL Mesh Leveling System. You will be able to tilt your mesh based on a 2x2 to 9x9 grid. That grid is fed into an incremental Least Squares Fit algorithm that @edwilliams16 designed to keep memory usage to a minimum. We don't even have to hold onto the full data set to do a 9x9 LSF probing of the bed!

That gets merged in at the same time the Smart Fill gets merged in and that is already developed, debugged AND tested.

@Roxy-3D
Copy link
Member

Roxy-3D commented Apr 26, 2017

@edwilliams16 Your algorithm is 'Real'. It is in the main line code base now!!!

THANK YOU FOR THE HELP!!!!

@thinkyhead
Copy link
Member

thinkyhead commented Apr 29, 2017

if we wait an extra week

That's not going to happen. I already have everything lined up and have been announcing Sunday's release in various high-visibility locations. Please hold any new work on UBL until 1.1.1 and keep focusing on basic spit and polish for tomorrow's release.

@bgort bgort deleted the ubl_progmem branch May 8, 2017 02:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants