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

[FR] Feed Rate Unit #18923

Closed
qwewer0 opened this issue Aug 5, 2020 · 7 comments
Closed

[FR] Feed Rate Unit #18923

qwewer0 opened this issue Aug 5, 2020 · 7 comments
Labels
C: Configuration T: Feature Request Features requested by users.

Comments

@qwewer0
Copy link
Contributor

qwewer0 commented Aug 5, 2020

Description

Is there a way to make MANUAL_FEEDRATE in Configuration_adv.h to be in mm/s instead of mm/min, like it is for DEFAULT_MAX_FEEDRATE and MAX_FEEDRATE_EDIT_VALUES in Configuration.h?
Or to clarify the used unit.

e.g.
MANUAL_FEEDRATE { 50*60, 50*60, 4*60, 60 } -> MANUAL_FEEDRATE { 50, 50, 4, 1 }
or
Feedrates for manual moves along X, Y, Z, E from panel -> Feedrates (mm/min) for manual moves along X, Y, Z, E from panel

@boelle boelle added the T: Feature Request Features requested by users. label Aug 5, 2020
@sjasonsmith
Copy link
Contributor

I agree that this is confusing.

@thinkyhead
Copy link
Member

You cannot just change the units of an existing configuration option. It would need to be renamed (e.g., MANUAL_FEEDRATE_MM_S if new units are going to be used. And then I suppose the longstanding and venerable DEFAULT_MAX_FEEDRATE would need to be renamed to DEFAULT_MAX_FEEDRATE_MM_S…. And then it's no use. All the rest of the feedrate settings will need to be likewise changed.

But then… When you consider that the feedrate F parameter in all of our G-code is always in current-units-per-minute, then maybe all settings should really be converted to mm/m for overall consistency. And, there are several other G-codes which I am sure take a feedrate parameter in current-units-per-second, so I guess it would be pretty disruptive to change all of those.

In general, it seems that each feature was defined in an ad hoc way, as befit its author at the time it was implemented. In some cases users may find it easier to think in terms of mm/m because they set their movement speed, accelerations, and limits according to those numbers. The use of n*60 values in a feedrate setting is more about providing a coarse adjustment for those values, but perhaps they should just be set as whole numbers so they correlate with the common experience of F<feedrate> parameters and other settings like it which are expressed in units-per-minute…?

@qwewer0
Copy link
Contributor Author

qwewer0 commented Aug 11, 2020

Then the easiest option would be to just add the unit in the comment to notify the user.
#define MANUAL_FEEDRATE { 50*60, 50*60, 4*60, 60 } // Feedrates (mm/min) for manual moves along X, Y, Z, E from panel

Edit: Submitted a PR, if this would be the way to go.

@thinkyhead
Copy link
Member

Thanks! All items that aren't clear should at least have a comment about what their units are.

@AnHardt
Copy link
Member

AnHardt commented Aug 11, 2020

Yes. But here the notation with "x*60" already was a strong indicator.

@qwewer0
Copy link
Contributor Author

qwewer0 commented Aug 11, 2020

Better safe than sorry.

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked and limited conversation to collaborators Oct 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
C: Configuration T: Feature Request Features requested by users.
Projects
None yet
Development

No branches or pull requests

5 participants