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

UBL: do not skip grid display on mesh edit #17897

Merged
merged 1 commit into from
May 8, 2020

Conversation

tpruvot
Copy link
Contributor

@tpruvot tpruvot commented May 6, 2020

The whole grid is skipped (black screen)
if G28 is needed and also on point selection.

Only avoid double movement commands, may skip some X/Y moves
but its made after, on point selection, before the Z probe...

The whole grid is skipped (black screen)
if G28 is needed and also on point selection.

Only avoid double movement commands, may skip some X/Y moves
but its made after, on point selection, before the Z probe...
@thinkyhead
Copy link
Member

thinkyhead commented May 8, 2020

One of the big TODO items is to look at code that takes control of the LCD during procedures and use what we have developed so far as guidance to build a consistent "wizard" API that all step-by-step procedures can use. Maybe it's good to show the mesh display during the whole move, or maybe it's better to put up a modal "Moving…" message to make it clear that no interaction is possible. Things like this can be standardized more.

Working on UI refinements and making the HALs and core features bug free seems to be our main pandemic response.

@tpruvot
Copy link
Contributor Author

tpruvot commented May 8, 2020

during the G28, it was indeed replaced by a long "Homing XYZ" but not on point selection, in both cases the grid was not displayed at all.. (black screen, no message) which was very confusing.

@thinkyhead
Copy link
Member

thinkyhead commented May 10, 2020

I had tried to add an intermediate screen, but the code for UBL and its mesh editing is not quite like the rest of Marlin, so I had to put off the job for later. I believe it can be managed. It's just unusual to put the machine into a modal state and block the G-code queue for such a long time inside of a G-code. Any mesh editing g-code needs to work in a manner more like MBL's G29 or ABL's G29 with PROBE_MANUALLY, so the command quickly returns control back to the main loop.

@Roxy-3D
Copy link
Member

Roxy-3D commented May 10, 2020

It's just unusual to put the machine into a modal state and block the G-code queue for such a long time inside of a G-code.

There are plenty of examples of other GCodes taking over the machine for their purposes. For example, M303. Nothing else works while that is active.

The thing is, editing the mesh is something that is done to setup the machine, just like M303. It probably isn't worth the extra complexity to make it possible that other GCodes can be sent to the printer while that is busy doing its work.

@thinkyhead
Copy link
Member

I agree that some procedures are best not interrupted by other UI activities, and taking over the screen and blocking the G-code buffers are not the worst things. But to qualify that, I hope to get closer to a more stateful Marlin so that we are able to return to the main loop to keep the G-code processor active and the UI responsive more easily.

The emergency parser and some other elements are designed to deal with Marlin's lack of state, so we can break into the flow at a low level, because the means to do so at a higher level is blocked in many ordinary cases. Of course, M108 and M112 can also deal with things like a full buffer, so EP is indispensable at this point.

I think mostly I'm anxious to be able to have the firmware work more like an app, with fewer ad hoc solutions and a some more APIs to fill in the gray areas and remove the mystery of how to do all the usual things:

  • Put up some type of screen (modal alert, modal select screen, custom…)
  • Get the answer back from the screen selection (or cancel)
  • Standard cancel methods like double-click, click-hold as part of wait loops

If I write up some guidance on current methods and spend some time commenting the existing code to sort that out, something will drop out of that exercise. I see patterns, but it's not always clear how to turn those patterns into something more systematic.

I shall turn to Discord and see if the other code nerds have thoughts…

@Roxy-3D
Copy link
Member

Roxy-3D commented May 10, 2020

I agree that some procedures are best not interrupted by other UI activities, and taking over the screen and blocking the G-code buffers are not the worst things. But to qualify that, I hope to get closer to a more stateful Marlin so that we are able to return to the main loop to keep the G-code processor active and the UI responsive more easily.

Well... Here is another perspective... Originally G26 was written for UBL. But it made sense to change the mesh declarations so any mesh based bed leveling system could use it. And the same goes for saving the mesh at the end of the EEPROM. It just made sense to push everything in the same direction.

Probably the same thing applies here. It may make sense to re-write the Interactive Mesh Edit function so it can be used with any of the mesh based leveling systems. And if that was done, it would be fairly simple to make it optional and only pull it into the image if the user wanted it and had room for it. At that point... it probably would not be too hard to make it more friendly to the serial port and allow other commands to come in while it is working its magic.

@thinkyhead
Copy link
Member

There is a better solution for the EEPROM conundrum. https://github.com/MarlinFirmware/Marlin/projects

@thinkyhead
Copy link
Member

It may make sense to re-write the Interactive Mesh Edit function

Yeah, in that vein I would like to break down procedures like mesh edit to pieces that are easy to use for other procedures. Like, for example, I need to make a little setup wizard to measure your probe's Z offset by having you adjust the nozzle to touch the bed, then probing that spot. The little bit of code to adjust the nozzle is used in so many places, but all in slightly different ways. That's one Lego® brick that should be part of an API that works anywhere (at least, within any G-code handler).

It can be more consistent and cleaner to "take over" the LCD controller from within G-codes and run a loop, so internal-only G-codes should be considered for any menu items that need to initiate wizards and other procedures.

@Roxy-3D
Copy link
Member

Roxy-3D commented May 10, 2020

I need to make a little setup wizard to measure your probe's Z offset by having you adjust the nozzle to touch the bed, then probing that spot. The little bit of code to adjust the nozzle is used in so many places, but all in slightly different ways. That's one Lego® brick that should be part of an API that works anywhere (at least, within any G-code handler).

Yes. Agreed. It would be good to shift this repeatedly used procedure to a common implementation.

vgadreau pushed a commit to vgadreau/Marlin that referenced this pull request May 29, 2020
jmp0x0000 pushed a commit to jmp0x0000/Marlin that referenced this pull request Aug 7, 2020
njibhu pushed a commit to njibhu/Marlin that referenced this pull request Aug 24, 2020
@tpruvot tpruvot deleted the ubl_lcd_grid branch December 5, 2020 06:34
HairingX pushed a commit to HairingX/Marlin that referenced this pull request Jun 16, 2021
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.

3 participants