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

Stepsperunit parameters #189

Open
wants to merge 1 commit into
base: experimental
Choose a base branch
from

Conversation

felipesanches
Copy link
Contributor

This is what I consider a better way of configuring axis scaling and extruder parameters.

I have left the hard-coded magic numbers because I still dont know which specific parameters where used to obtain those values. Please consider expressing all those configurations by defining the following attributes, so that they are easier to tweak/understand:

  • steps_per_revolution
  • microstepping
  • belt_pitch
  • pulley_teeth
  • extruder_gears_ratio
  • bolt_diameter
  • z_thread_distance

@kliment
Copy link
Owner

kliment commented Jun 15, 2012

Hey,

I don't like this because it makes the config file huge and confusing. Maybe if the machine definitions were at the end of the file it would be better. I think the current one line config is better for that. I much rather have 4 magic numbers than 7*n.

@felipesanches
Copy link
Contributor Author

The purpose of this proposal is to replace the magic numbers by numbers that have a comprehensible meaning. Off course we can refactor it so that it becomes less confusing. Implementing the calculation of steps_per_mm in the source by using macros is, in my opinion, much better than keeping the formula in a wiki page and expecting the users to visit that page, calculate the values by hand and paste the results in the Configuration.h file. Using macros won't affect the size of the resulting binary, won't impact performance since the calculation is done in compile time and will benefit us by adding better semantics to the source.

I will try to work on a second proposal, cleaner than this one, and then I'll probably send you another pull request for your consideration. This message is just to let you know the rationale behind my proposal and to get some more feedback from you.

@kliment
Copy link
Owner

kliment commented Jun 18, 2012

I would prefer having the formula in comments next to the numbers, and
keeping the numbers as they are. Having commented out premade configs
for existing machines is okay, but I don't see why they should take more
than one line each. Given the variety of mechanical configurations in
existence it adds complexity to the config to do it with defines. I
don't like it. We should definitely put the formulas in there, but as
documentation, not as code.

On 06/18/2012 02:07 PM, felipesanches wrote:

The purpose of this proposal is to replace the magic numbers by numbers that have a comprehensible meaning. Off course we can refactor it so that it becomes less confusing. Implementing the calculation of steps_per_mm in the source by using macros is, in my opinion, much better than keeping the formula in a wiki page and expecting the users to visit that page, calculate the values by hand and paste the results in the Configuration.h file. Using macros won't affect the size of the resulting binary, won't impact performance since the calculation is done in compile time and will benefit us by adding better semantics to the source.

I will try to work on a second proposal, cleaner than this one, and then I'll probably send you another pull request for your consideration. This message is just to let you know the rationale behind my proposal and to get some more feedback from you.


Reply to this email directly or view it on GitHub:
#189 (comment)

@felipesanches
Copy link
Contributor Author

If the problem is space, we could keep each machine config in its own config file, so that it's both easy to understand&tweak and not cluttered in a single giant file.

Also, keeping all parameters explicit helps one better understand the characteristics of each machine. By looking at
#define _AXIS_STEP_PER_UNIT {104.987, 104.987, 4545.4544, 1487}
I have no clue what's the actual config of the MakerGear Hybrid. I think source code should be explicit: it should be as semantic as possible also serving as hardware documentation.

By keeping these in separate files we can achieve that: it would be clean to read the main config file, and the details would be there in the specific machine config files. Win-Win.

@kliment
Copy link
Owner

kliment commented Jun 18, 2012

The problem is not space but complexity.
Some people have leadscrew-driven axes. Some people have direct drive
extruders. Some people have belt or rack/pinion axes. Measuring the
effective diameter of a hobbed bolt is a terrible pain and depends on
idler tension. Making every configuration possible in the code makes it
extremely messy. I also do not want to have a large number of different
files for different machines, with overrides and multipliers and
selection indices. Put the machine configuration description in as a
comment AND the magic numbers as code, and have the formulas in a
comment separately to enable people to calculate their own for different
machines. I don't mind explicit code, but I do mind a halfway there
model of reality that dramatically increases complexity.

On 06/18/2012 03:14 PM, felipesanches wrote:

If the problem is space, we could keep each machine config in its own config file, so that it's both easy to understand&tweak and not cluttered in a single giant file.

Also, keeping all parameters explicit helps one better understand the characteristics of each machine. By looking at
#define _AXIS_STEP_PER_UNIT {104.987, 104.987, 4545.4544, 1487}
I have no clue what's the actual config of the MakerGear Hybrid. I think source code should be explicit: it should be as semantic as possible also serving as hardware documentation.

By keeping these in separate files we can achieve that: it would be clean to read the main config file, and the details would be there in the specific machine config files. Win-Win.


Reply to this email directly or view it on GitHub:
#189 (comment)

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.

2 participants