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

Fix UBL mesh editing #17670

Merged
merged 13 commits into from
Apr 28, 2020
Merged

Conversation

filippor
Copy link
Contributor

fix move on ubl mesh edit
x_plot , y_plot can be negative when move backward (see _lcd_ubl_output_map_lcd when step_scaler <0)

planner.quick_stop(); cause crazy moviment out of working area on my configuration

@filippor
Copy link
Contributor Author

to fix crazy movment how can I create a critical section?
void Planner::quick_stop() {

// Remove all the queued blocks. Note that this function is NOT
// called from the Stepper ISR, so we must consider tail as readonly!
// that is why we set head to tail - But there is a race condition that
// must be handled: The tail could change between the read and the assignment
// so this must be enclosed in a critical section

@GMagician
Copy link
Contributor

by disabling interrupts, some macro are still defined: CRITICAL_SECTION_START and CRITICAL_SECTION_END, these must be called before (START) and after (END) code that needs to be inside a critical section

@filippor
Copy link
Contributor Author

I tried with critical section but i makes Marlin freeze
I removed _lcd_hard_stop and do nothing if planner.movesplanned()
i think is safer

@GMagician
Copy link
Contributor

GMagician commented Apr 23, 2020

Not sure what/why planner.movesplanned is used there but I think that if it's present there is for sure a reason...

EDIT: movesplanned checks if some movement may be added into queue...if error raise it means that there is no space for any other movement to be enqueued. That's not good

@filippor
Copy link
Contributor Author

Before my change when planner.movesplanned calls Planner::quick_stop() that if I understand correctly clear the buffer

Now the function return without making changes and planning move it is not ideal but it works

The current does not work for my delta printer.

@thinkyhead
Copy link
Member

thinkyhead commented Apr 25, 2020

I've been looking at the menu code and whether this is the best approach. I need to install UBL on a printer and play around with the mesh editing interface, refine it until the behavior is consistent with expectations. Namely:

  • Select a mesh point on the screen with the wheel
  • Click to select a point and move the nozzle to it
  • Allow the selection of another mesh point during the move
  • If another point is clicked, queue a move to the that point too
  • If you click the point already selected, go to the edit screen…
  • (Editing is delayed until the mesh move completes)

@thinkyhead thinkyhead changed the title fix ubl Fix UBL mesh editing Apr 25, 2020
thinkyhead added a commit that referenced this pull request Apr 25, 2020
Most obvious part of #17670

Co-Authored-By: FilippoR <filippo.rossoni@gmail.com>
@thinkyhead
Copy link
Member

I've tried a different approach here. It uses ui.synchronize to prevent interaction until a move is done. See if it works for you. I'll be testing this after dinner.

@filippor
Copy link
Contributor Author

I test this patch and when enter in mesh edit te printer restart. Ill try to find the problem

@thinkyhead
Copy link
Member

I think that in order to use the ui.synchronize method the rest of the code has to be structured to go with it. At some point in future I want to standardize our native LCD API to support color DWIN / DGUS displays with encoders. The Ender 3 v2 code contains DWIN code that provides a great guide on how to draw on these displays.

Anyway, I'll clean this up and merge it, and I'll revisit again during the next LCD code scrub.

@filippor
Copy link
Contributor Author

Now it works thanks

@thinkyhead thinkyhead merged commit 9fa5119 into MarlinFirmware:bugfix-2.0.x Apr 28, 2020
jmp0x0000 pushed a commit to jmp0x0000/Marlin that referenced this pull request Aug 7, 2020
Most obvious part of MarlinFirmware#17670

Co-Authored-By: FilippoR <filippo.rossoni@gmail.com>
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
Most obvious part of MarlinFirmware#17670

Co-Authored-By: FilippoR <filippo.rossoni@gmail.com>
njibhu pushed a commit to njibhu/Marlin that referenced this pull request Aug 24, 2020
HairingX pushed a commit to HairingX/Marlin that referenced this pull request Jun 16, 2021
Most obvious part of MarlinFirmware#17670

Co-Authored-By: FilippoR <filippo.rossoni@gmail.com>
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