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

G35 assisted tramming improvements #20298

Conversation

qwewer0
Copy link
Contributor

@qwewer0 qwewer0 commented Nov 27, 2020

Description

  • Adding ASSISTED_TRAMMING_WAIT_POSITION to G35.
    This already existed, but worked only with the tramming wizard and not the G35 command.

  • Removing ASSISTED_TRAMMING_MENU_ITEM.
    (G35 assisted tramming improvements #20298 (comment))
    This only ran G35, which provides no value without being able to see the serial output.
    Users wishing for an interactive experience should use the wizard option.

  • Disables homing Z after the G35 comment
    There was no value in homing Z after executing G35, because at that point you have not yet adjusted the screws.
    Instead, simply invalidate Z, so that homing is required before next use.

Benefits

Easier screw adjustment after G35 is done, in particular for printers where the print head blocks screw access.

Related Issues

#20281

@qwewer0 qwewer0 marked this pull request as ready for review November 27, 2020 16:42
@thinkyhead thinkyhead force-pushed the G35_ASSISTED_TRAMMING_WAIT_POSITION branch from 8b17c43 to 4184f8a Compare November 27, 2020 22:01
Marlin/src/gcode/bedlevel/G35.cpp Outdated Show resolved Hide resolved
@sjasonsmith
Copy link
Contributor

@qwewer0 & @swissnorp it seems you are still working through some issues? Please reply when you have things worked out and think this is ready to review.

@qwewer0
Copy link
Contributor Author

qwewer0 commented Dec 12, 2020

@qwewer0 & @swissnorp it seems you are still working through some issues? Please reply when you have things worked out and think this is ready to review.

@sjasonsmith I think this is ready for review.
I tested it and it is working as intended.

@sjasonsmith sjasonsmith self-assigned this Dec 27, 2020
@sjasonsmith
Copy link
Contributor

sjasonsmith commented Dec 27, 2020

Sorry, this got a little lost in the pile and I never came back to it. Unfortunately I've now posted other changes which will conflict with this!

At this point I would prefer to wait for #20572 to be merged, then I can come back and resolve any issues it causes with this PR.

@sjasonsmith sjasonsmith added the Needs: Work More work is needed label Dec 27, 2020
@sjasonsmith
Copy link
Contributor

I actually ran across this missing capability when I was testing that other PR. It was completely non-obvious in the config file that it applied only to the wizard!

@sjasonsmith sjasonsmith linked an issue Dec 27, 2020 that may be closed by this pull request
qwewer0 and others added 10 commits December 29, 2020 12:10
Adding ASSISTED_TRAMMING_WAIT_POSITION to G35
ASSISTED_TRAMMING_WAIT_POSITION Comment Consistency
Z axis is alreagy set to be unhomed.
"If it doesn’t report information back to the screen it doesn’t provide any real value. It seems unlikely that people would want to monitor it over serial, but don’t also have a way to send G35 over serial."
@qwewer0 qwewer0 force-pushed the G35_ASSISTED_TRAMMING_WAIT_POSITION branch from 94c97dc to 965bf40 Compare December 29, 2020 11:17
@qwewer0
Copy link
Contributor Author

qwewer0 commented Dec 29, 2020

On the latest Pronterface they fixed the issue (kliment/Printrun#1103) with Clockwise, Counter-Clockwise not showing up correctly. Should we change it back or leave it as it is right now?
(d02d7a9)

@sjasonsmith
Copy link
Contributor

Should we change it back or leave it as it is right now?

I don't know, but I don't think this change should be dependent on that.

@sjasonsmith sjasonsmith changed the title G35 assisted tramming wait position G35 assisted tramming improvements Jan 1, 2021
Copy link
Contributor

@sjasonsmith sjasonsmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took the liberty of updating the title/description to make the reasoning for the changes apparent at a glance.

I think these are great changes. These items were all confusing while I was working on the code to validate the selected tramming points at build-time.

Aside from making G35 more useful, I think they will also help reduce some new-user confusion.

@sjasonsmith sjasonsmith added S: Please Merge and removed Needs: Work More work is needed labels Jan 1, 2021
@sjasonsmith
Copy link
Contributor

@qwewer0 could you add a sanity check for the removed option?

@qwewer0
Copy link
Contributor Author

qwewer0 commented Jan 1, 2021

@qwewer0 could you add a sanity check for the removed option?

Done, could you check it, please?

@thinkyhead thinkyhead merged commit e685950 into MarlinFirmware:bugfix-2.0.x Jan 2, 2021
@qwewer0
Copy link
Contributor Author

qwewer0 commented Jan 2, 2021

@thinkyhead Thanks for the cleanup, looks good and logical. 👍

@qwewer0 qwewer0 deleted the G35_ASSISTED_TRAMMING_WAIT_POSITION branch January 2, 2021 08:36
TheMichalcinOfficial added a commit to TheMichalcinOfficial/Marlin that referenced this pull request Jan 4, 2021
* [cron] Bump distribution date (2020-12-31)

* SPI and pins cleanup

* [cron] Bump distribution date (2021-01-01)

* Prefix SD SPI pins (SCK, MISO, MOSI, SS) (MarlinFirmware#20606)

Co-authored-by: Scott Lahteine <[email protected]>

* Fix PARKING_EXTRUDER homing with solenoid (MarlinFirmware#20473)

* Fix CHAMBER_FAN_MODE 0 build (MarlinFirmware#20621)

* [cron] Bump distribution date (2021-01-02)

* Fix UBL mesh edit delta moves (MarlinFirmware#20620)

Co-authored-by: Scott Lahteine <[email protected]>

* Fix //action prefix (MarlinFirmware#20600)

* Assisted Tramming improvements (MarlinFirmware#20298)

* Check for misplaced configs on build (MarlinFirmware#20599)

Co-authored-by: Scott Lahteine <[email protected]>

* Fix a comment (MarlinFirmware#20629)

* Document, adjust some homing code

* Improved bootscreen animation

* [cron] Bump distribution date (2021-01-03)

* Homing code followup (MarlinFirmware#20632)

Patching a87e519

* Animated boot followup

* Add ALL_AXES manual move for UBL mesh editing

Co-Authored-By: Jason Smith <[email protected]>

MarlinFirmware#20620

* Creality 4.2.10 board (MarlinFirmware#20647)

* Update Italian language (MarlinFirmware#20663)

* [cron] Bump distribution date (2021-01-04)

* Fix thermal error protection, reporting (MarlinFirmware#20655)

* Rename FTDI EVE screen data structs

* Fix SD SPI Speed override, FTDI mesh edit (MarlinFirmware#20657)

Co-authored-by: Scott Lahteine <[email protected]>

* Fix IDEX reboot on travel after G28 X (MarlinFirmware#20654)

* Fix delayed_move_time elapsed test

* Move duplication_e_mask

Co-authored-by: thinkyhead <[email protected]>
Co-authored-by: Scott Lahteine <[email protected]>
Co-authored-by: Victor Oliveira <[email protected]>
Co-authored-by: zeleps <[email protected]>
Co-authored-by: Jason Smith <[email protected]>
Co-authored-by: ellensp <[email protected]>
Co-authored-by: qwewer0 <[email protected]>
Co-authored-by: Giuliano Zaro <[email protected]>
Co-authored-by: Marcio T <[email protected]>
Co-authored-by: InsanityAutomation <[email protected]>
tharts pushed a commit to tharts/Marlin that referenced this pull request Jan 6, 2021
@o5
Copy link

o5 commented Jan 7, 2021

Cannot build after rebase with this change :/

Compiling .pio/build/STM32F103RC_btt_USB/src/src/gcode/bedlevel/abl/M421.cpp.o
In file included from Marlin/src/feature/tramming.cpp:27:0:
Marlin/src/feature/tramming.h:36:36: error: static assertion failed: TRAMMING_POINT_XY point 0 is not reachable with the default NOZZLE_TO_PROBE offset and PROBING_MARGIN.
 #define VALIDATE_TRAMMING_POINT(N) static_assert(N >= G35_PROBE_COUNT || Probe::build_time::can_reach(screws_tilt_adjust_pos[N]), \
                                    ^
Marlin/src/feature/tramming.h:38:1: note: in expansion of macro 'VALIDATE_TRAMMING_POINT'
 VALIDATE_TRAMMING_POINT(0); VALIDATE_TRAMMING_POINT(1); VALIDATE_TRAMMING_POINT(2); VALIDATE_TRAMMING_POINT(3); VALIDATE_TRAMMING_POINT(4);
 ^~~~~~~~~~~~~~~~~~~~~~~
Marlin/src/feature/tramming.h:36:36: error: static assertion failed: TRAMMING_POINT_XY point 3 is not reachable with the default NOZZLE_TO_PROBE offset and PROBING_MARGIN.
 #define VALIDATE_TRAMMING_POINT(N) static_assert(N >= G35_PROBE_COUNT || Probe::build_time::can_reach(screws_tilt_adjust_pos[N]), \
                                    ^
Marlin/src/feature/tramming.h:38:85: note: in expansion of macro 'VALIDATE_TRAMMING_POINT'
 VALIDATE_TRAMMING_POINT(0); VALIDATE_TRAMMING_POINT(1); VALIDATE_TRAMMING_POINT(2); VALIDATE_TRAMMING_POINT(3); VALIDATE_TRAMMING_POINT(4);
                                                                                     ^~~~~~~~~~~~~~~~~~~~~~~
*** [.pio/build/STM32F103RC_btt_USB/src/src/feature/tramming.cpp.o] Error 1
In file included from Marlin/src/gcode/bedlevel/G35.cpp:43:0:
Marlin/src/gcode/bedlevel/../../feature/tramming.h:36:36: error: static assertion failed: TRAMMING_POINT_XY point 0 is not reachable with the default NOZZLE_TO_PROBE offset and PROBING_MARGIN.
 #define VALIDATE_TRAMMING_POINT(N) static_assert(N >= G35_PROBE_COUNT || Probe::build_time::can_reach(screws_tilt_adjust_pos[N]), \
                                    ^
Marlin/src/gcode/bedlevel/../../feature/tramming.h:38:1: note: in expansion of macro 'VALIDATE_TRAMMING_POINT'
 VALIDATE_TRAMMING_POINT(0); VALIDATE_TRAMMING_POINT(1); VALIDATE_TRAMMING_POINT(2); VALIDATE_TRAMMING_POINT(3); VALIDATE_TRAMMING_POINT(4);
 ^~~~~~~~~~~~~~~~~~~~~~~
Marlin/src/gcode/bedlevel/../../feature/tramming.h:36:36: error: static assertion failed: TRAMMING_POINT_XY point 3 is not reachable with the default NOZZLE_TO_PROBE offset and PROBING_MARGIN.
 #define VALIDATE_TRAMMING_POINT(N) static_assert(N >= G35_PROBE_COUNT || Probe::build_time::can_reach(screws_tilt_adjust_pos[N]), \
                                    ^
Marlin/src/gcode/bedlevel/../../feature/tramming.h:38:85: note: in expansion of macro 'VALIDATE_TRAMMING_POINT'
 VALIDATE_TRAMMING_POINT(0); VALIDATE_TRAMMING_POINT(1); VALIDATE_TRAMMING_POINT(2); VALIDATE_TRAMMING_POINT(3); VALIDATE_TRAMMING_POINT(4);

Unfortunately, I don't understand completely this feature so I'm not able to fix assertion. Could someone help me? There is my sources marlin, marlin-configurations.

@qwewer0
Copy link
Contributor Author

qwewer0 commented Jan 7, 2021

@o5 #20572

@o5
Copy link

o5 commented Jan 7, 2021

OK, I discovered that I enabled this option when reading some Reddit guide and (as most of others options... in that time, I promise!) and I didn't exactly know what I did :) So time to change it!

Looked into documentation and some explanation in youtube video but still cannot confidently answer my question: Is this calibration necessary for me if I'm using auto bed leveling with fixed probe before every print using G29? I think I should comment out ASSISTED_TRAMMING.

@qwewer0
Copy link
Contributor Author

qwewer0 commented Jan 7, 2021

@o5 G35 can help you tram your bed. It is up to you if you want to use it or not, you can tram your bed with the paper method too.

dpreed pushed a commit to dpreed/Marlin_2.0.x that referenced this pull request Feb 5, 2021
kpishere pushed a commit to kpishere/Marlin that referenced this pull request Feb 19, 2021
zillarob pushed a commit to zillarob/Marlin that referenced this pull request Feb 25, 2021
W4tel-BiDi pushed a commit to W4tel-BiDi/Marlin that referenced this pull request Apr 5, 2021
thinkyhead pushed a commit to thinkyhead/Marlin that referenced this pull request Apr 29, 2021
thinkyhead pushed a commit that referenced this pull request Apr 30, 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.

[FR] G35 - Specifiable End Position
5 participants