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

[2.0.x] Make MBL work more like PROBE_MANUALLY #8575

Merged
merged 1 commit into from
Nov 27, 2017

Conversation

thinkyhead
Copy link
Member

See also #8574

Fix #7912 and Fix #7491 by making MBL's LCD_BED_LEVELING work in the same manner as PROBE_MANUALLY.

@thinkyhead thinkyhead merged commit 47099af into MarlinFirmware:bugfix-2.0.x Nov 27, 2017
@thinkyhead thinkyhead deleted the kuru_to_upstream branch November 27, 2017 18:55
thinkyhead added a commit to thinkyhead/Marlin that referenced this pull request Nov 27, 2017
@@ -102,7 +90,7 @@ void GcodeSuite::G29() {
case MeshStart:
mbl.reset();
mbl_probe_index = 0;
enqueue_and_echo_commands_P(PSTR("G28\nG29 S2"));
enqueue_and_echo_commands_P(lcd_wait_for_move ? PSTR("G29 S2") : PSTR("G28\nG29 S2"));
Copy link
Contributor

@fiveangle fiveangle Dec 3, 2017

Choose a reason for hiding this comment

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

MBL does not require LCD, so this breaks printers w/o one

Also, this desperately needs a travis test as it comes up like every couple hundred commits 😝

Marlin/src/gcode/bedlevel/mbl/G29.cpp: In static member function 'static void GcodeSuite::G29()':
Marlin/src/gcode/bedlevel/mbl/G29.cpp:93:35: error: 'lcd_wait_for_move' was not declared in this scope
enqueue_and_echo_commands_P(lcd_wait_for_move ? PSTR("G29 S2") : PSTR("G28\nG29 S2"));

@Roxy-3D
Copy link
Member

Roxy-3D commented Dec 3, 2017

Also, this desperately needs a travis test as it comes up like every hundred commits 😝

I wonder if we could have two sets of Travis tests... And if so... We would put this test into the 'More Complete' set?

@fiveangle
Copy link
Contributor

I think testing with and w/o these should be part of basic tests wherever supported:

  • Panel (display + controller)
  • SD

The above example illustrates this.

But I agree with you that a more comphrensive set of tests would be nice. I think one that iterates through every example config would catch a lot of regressions that usually take weeks to be worked through in the tracker.

I looked at doing this back in spring but didn't find an easy (e.g. 5 min I spent ;) solution. It's relatively easy to break up the existing tests to parallelize them, but that doesn't really help much when the setup time on the Precise travis instance is between 4 and 10 minutes: it just spins travis resources w/o really lowering the test time.

@Roxy-3D
Copy link
Member

Roxy-3D commented Dec 3, 2017

I wouldn't even be opposed to making the extended Travis tests User Name related or perhaps even Pull Request number related. If once every 25 pull requests, we do an extended Travis Test... Nothing will go unnoticed for that long.

thinkyhead added a commit that referenced this pull request Dec 5, 2017
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