-
-
Notifications
You must be signed in to change notification settings - Fork 19.3k
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
PWM_MOTOR_CURRENT Flag #3131
PWM_MOTOR_CURRENT Flag #3131
Conversation
…nfiguration_adv.h similar to DIGIPOT_MOTOR_CURRENT
You cannot change the drivers on the Mini RAMBo. The motors used will dictate changing the PWM values, so this is definitely a setting that could be different for everyone (12 or 24V, full size or half stack, etc). Much like the full-sized RAMBo having a current control stanza I believe the Mini should also have one - especially because of its inclusion in popular printers like the Prusa i3 and LulzBot Mini. |
Sorry for being daft. You are correct! I was confusing this with setting the microsteps – not something you would do with PWM anyway. |
#ifndef PWM_MOTOR_CURRENT | ||
#define PWM_MOTOR_CURRENT DEFAULT_PWM_MOTOR_CURRENT | ||
#endif | ||
const int motor_current_setting[3] = PWM_MOTOR_CURRENT; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@thinkyhead it seems this needs to be static constexpr int motor_current_setting[3] = PWM_MOTOR_CURRENT;
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting. What is the problem that adding those specifiers solves?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@thinkyhead this line is now in stepper.h#L137. This would fix a compilation error 😆 ; the error being:
In file included from ./Configuration.h:1235:0,
from ./Marlin.h:44,
from ./stepper.cpp:47:
./stepper.h: In static member function ‘static void Stepper::digipot_init()’:
./Configuration_adv.h:346:44: error: invalid use of member ‘Stepper::motor_current_setting’ in static member function
#define PWM_MOTOR_CURRENT {1300, 1630, 1250} // Values in milliamps
^
./stepper.h:137:44: note: in expansion of macro ‘PWM_MOTOR_CURRENT’
const int motor_current_setting[3] = PWM_MOTOR_CURRENT;
^
./stepper.cpp:1009:26: error: from this location
digipot_current(0, motor_current_setting[0]);
^
In file included from ./Configuration.h:1235:0,
from ./Marlin.h:44,
from ./stepper.cpp:47:
./Configuration_adv.h:346:44: error: invalid use of member ‘Stepper::motor_current_setting’ in static member function
#define PWM_MOTOR_CURRENT {1300, 1630, 1250} // Values in milliamps
^
./stepper.h:137:44: note: in expansion of macro ‘PWM_MOTOR_CURRENT’
const int motor_current_setting[3] = PWM_MOTOR_CURRENT;
^
./stepper.cpp:1013:26: error: from this location
digipot_current(1, motor_current_setting[1]);
^
In file included from ./Configuration.h:1235:0,
from ./Marlin.h:44,
from ./stepper.cpp:47:
./Configuration_adv.h:346:44: error: invalid use of member ‘Stepper::motor_current_setting’ in static member function
#define PWM_MOTOR_CURRENT {1300, 1630, 1250} // Values in milliamps
^
./stepper.h:137:44: note: in expansion of macro ‘PWM_MOTOR_CURRENT’
const int motor_current_setting[3] = PWM_MOTOR_CURRENT;
^
./stepper.cpp:1017:26: error: from this location
digipot_current(2, motor_current_setting[2]);
Static member functions can only access static data members.
Here's how Wikipedia explains constant expressions:
C++ has always had the concept of constant expressions. These are expressions such as 3+4 that will always yield the same results, at compile time and at run time. Constant expressions are optimization opportunities for compilers, and compilers frequently execute them at compile time and hardcode the results in the program. Also, in several places, the C++ specification requires using constant expressions. Defining an array requires a constant expression, and enumerator values must be constant expressions.
constexpr
was introduced in the C++11 standard.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like we could use constexpr
in more places where const
is currently used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems not to be exactly the same thing..
const
declares an object as constant. This implies a guarantee that, once initialized, the value of that object won't change, and the compiler can make use of this fact for optimizations. It also helps prevent the programmer from writing code that modifies objects that were not meant to be modified after initialization.constexpr
declares an object as fit for use in what the Standard calls constant expressions. But note thatconstexpr
is not the only way to do this.
When applied to functions the basic difference is this:
const
can only be used for non-static member functions, not functions in general. It gives a guarantee that the member function does not modify any of the non-static data members.constexpr
can be used with both member and non-member functions, as well as constructors. It declares the function fit for use in constant expressions. The compiler will only accept it if the function meets certain criteria (7.1.5/3,4), most importantly (†):- The function body must be non-virtual and extremely simple: Apart from typedefs and static asserts, only a single
return
statement is allowed. In the case of a constructor, only an initialization list, typedefs and static assert are allowed. (= default
and= delete
are allowed, too, though.) - The arguments and the return type must be literal types (i.e., generally speaking, very simple types, typically scalars or aggregates)
- The function body must be non-virtual and extremely simple: Apart from typedefs and static asserts, only a single
This PR – adding on to #2893 and answering #3125 – adds a new setting to the
Configuration_adv.h
to override the motor current PWM values previously set in the pins file. This needs some discussion, since we don't know any specific cases when it might be useful to do this, as these values properly belong in thepins_MYBOARD.h
files. Perhaps if you were using a different brand of stepper driver on your MiniRAMBo that takes different PWM values….?Corresponds to MarlinFirmware/MarlinDev#318