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

Change XY_PROBE_SPEED format to (x*60) #18997

Merged
merged 3 commits into from Aug 22, 2020
Merged

Change XY_PROBE_SPEED format to (x*60) #18997

merged 3 commits into from Aug 22, 2020

Conversation

qwewer0
Copy link
Contributor

@qwewer0 qwewer0 commented Aug 11, 2020

Description

Changing XY_PROBE_SPEED value format to match the other mm/m feedrate formats in the configurations.

Benefits

Feedrate format uniformity.

Related Issues

None

Changing XY_PROBE_SPEED value format to match the other mm/m formats in the configurations.
@thinkyhead
Copy link
Member

The "natural" unit for feedrates is the mm/m value used with the F parameter, so mm/m feedrates that are now expressed in whole mm/m values don't need to be re-expressed in mm/s. The reason for using x*60 inside of the manual feedrates array was only to make it easier to see the relative proportional speeds between the elements of the array, not because mm/s is in any way a better or more comprehensible unit for feedrates.

@qwewer0
Copy link
Contributor Author

qwewer0 commented Aug 14, 2020

My idea was to use the same format for XY_PROBE_SPEED as for HOMING_FEEDRATE_XY, HOMING_FEEDRATE_Z, MANUAL_FEEDRATE.

If we’re already here, may I ask why DEFAULT_MAX_FEEDRATE is in mm/s?

@AnHardt
Copy link
Member

AnHardt commented Aug 14, 2020

For historical and (backward) compatibility reasons we have to use mm/minute for the F-parameter. (maybe inch/minute in inch-mode). For the average feedrates used in early CNC-machines that made perfect sense.
With the faster speeds our 3D-Printers are running at, at least some of us, have a better feeling/intuition for speeds in mm/second.
For Marlin internal calculations we one day decided to use mm/seconds. At least for a while we targeted to write down our configuration parameters in mm/second, but have not been strict with that (like always).
On the other hand the "x*60"-notation for values in mm/minute is very attractive because the users with a good 'feeling' for mm/s can easily throw in their numbers and those with a better 'intuition' for mm/minute can easily omit the "*60" . The same would be true for "x/60.0". So in the config-files this is not critical.

Adjusting by g-code is much less satisfying - we can't calculate but only read numbers.

On the long term a calculating parser could be useful.
On the short term we could make a mode-setting for the speeds/feedrates used in g-code:

  • TRADITIONAL - mixed - like it is today
  • MM_PER_MINUTE - all speeds in mm/minute for all g-codes and F
  • MM_PER_SECOND - all speeds in mm/second for all g-codes except F
  • STRICT_MM_PER_SECOND - all speeds in mm/second including F

Internal representation should stay in mm/s.
Don't know what to do with M503. It, at least, needs a hint about the mode.

@AnHardt
Copy link
Member

AnHardt commented Aug 14, 2020

For the derived units, like accelerations, mm/minute becomes really ugly. I would not like to see mm/minute²

@qwewer0
Copy link
Contributor Author

qwewer0 commented Aug 14, 2020

Yeah, a uniformity of mm/m, mm/m² or mm/s, mm/s² would be nice, but I know it isn't an easy/quick task to change it all.

IMO, the ideal option would be at first

MM_PER_SECOND - all speeds in mm/second for all g-codes except F

then in the distant future to have it as

STRICT_MM_PER_SECOND - all speeds in mm/second including F

with a legacy option in the configurations to have the F parameter in mm/m.

@AnHardt
Copy link
Member

AnHardt commented Aug 14, 2020

mm/m

mm/m is millimetre/meter = 1000. Always!
Maybe mm/min is acceptable.

@qwewer0
Copy link
Contributor Author

qwewer0 commented Aug 14, 2020

mm/m is millimetre/meter = 1000. Always!

I think mm/m is short for mm/min in the configurations.

@AnHardt
Copy link
Member

AnHardt commented Aug 14, 2020

Then take it as an error-report.

@thinkyhead thinkyhead merged commit 434e43c into MarlinFirmware:bugfix-2.0.x Aug 22, 2020
@qwewer0 qwewer0 deleted the bugfix-2.0.x branch August 22, 2020 07:15
susisstrolch pushed a commit to susisstrolch/Marlin that referenced this pull request Aug 24, 2020
… into bugfix-2.0.x

* 'bugfix-2.0.x' of https://github.com/MarlinFirmware/Marlin: (33 commits)
  Minor cleanup w/r/t LEDs
  TFT32 for MKS Robin Nano 1.2 (MarlinFirmware#19031)
  [cron] Bump distribution date (2020-08-24)
  Feedrate comment (MarlinFirmware#19116)
  Neopixel => NeoPixel
  Fix up conditions, comments
  Fix DUET_SMART_EFFECTOR
  Consistent static/value item macros
  [cron] Bump distribution date (2020-08-23)
  Fix probing margin sanity-check
  PGMSTR constexpr => const
  NOZZLE_CLEAN_NO_Y (MarlinFirmware#18870)
  Change XY_PROBE_SPEED format to (x*60) (MarlinFirmware#18997)
  Reformat pins files
  Permit ST7789V orientation override (MarlinFirmware#19044)
  Prefer Servo AVR timer4 over 3 (MarlinFirmware#19025)
  DIGIPOT_I2C pins for SMOOTHIEBOARD (MarlinFirmware#19098)
  Translatable strings on Ender-3 V2 DWIN (MarlinFirmware#19053)
  HIGH/LOW naming of pin state settings (MarlinFirmware#19089)
  Update copy_marlin_variant_to_framework.py
  ...
albertogg pushed a commit to albertogg/Marlin that referenced this pull request Aug 31, 2020
thinkyhead pushed a commit to thinkyhead/Marlin that referenced this pull request Sep 2, 2020
vgadreau pushed a commit to vgadreau/Marlin that referenced this pull request Dec 9, 2020
kageurufu pushed a commit to CR30-Users/Marlin-CR30 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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants