Open cedricwritescode opened 11 months ago
cedricwritescode
commented
11 months ago I think it should be possible for the the absolute current position to be negative as well (uint64_t to int64_t). You reference it to a specific location using resetAbsolute() and may move to the left.
eldendiss
commented
11 months ago I will try to draft class diagram so we can discuss the changes before i'll start working on it.
As for the initialization, I think that you are right, more so with the possibility of more configuration options being added in the future. Using struct will be more clear and structured approach, while keeping the ESP-IDF idea of initialization.
eldendiss
commented
11 months ago I'm thinking something along this way. DendoStepper class would take care of pulse generation, profile calculation and bare-metal interaction with stepper. DendoStepperLinearAxis would then control movement along linear axis, directly in milimeters. I also want to add RadialAxis in order to be able to control shaft position directly in degrees/radians. DendoStepper would need to be initialized separately and then it will be passed as a constructor parameter to desired Axis class.
Additionally, I'm looking into preparing the API for an S-curve profile, drawing inspiration from this article
I'm open to any thoughts or suggestions you might have on this approach.
cedricwritescode
commented
11 months ago I think your idea with the different movement profiles would be great. Although, I'm trying to build a multi-axis positioning system (i.e. CNC machine) and would need a different motion planning (like non-constant velocity movement). The purpose of a motion profile for a stepper motor is to control the step frequency at any given step, regardless of the desired movement or the unit (i.e. steps, mm or radians). Therefore, the planner should get called in the ISR.
The stepper (or linear or radial) needs to set parameters of the movement, of course (using the movement_config_t struct). I think it will be easier to implement further movement planners in a backwards-compatible way.
I think the radial, and linear motion types would greatly benefit from inheritance.
However, I am wondering if a template interface might be beneficial at this point. I'm sure there are countless ways of representing the components, with only minor differences.
digiexchris
commented
11 months ago I'm not certain about calling the planner in the ISR. I've seen problems in other libraries with that method Bresenham's algorithm in the planner for each ISR iteration, and emitting a pulse according to the generated coordinates for that axis. It would generates unequally spaced steps if one of the axis were not moving at even pulse intervals. There's a large chance a stepper driver will miss a step if the instantaneous pulse width calculation generates a pulse that is too narrow or too close to the next step, when coordinating two axis moving together at the same time unless a closed loop stepper was used.
I feel that generating a number of steps in advance for multiple axis at the same time is the better strategy, and leaving the stepper class to just execute a queue of groups of generated steps. Marlin does this for 3d printers, and GRBL's esp32 port might be a good example also.
https://github.com/bdring/Grbl_Esp32/blob/main/Grbl_Esp32/src/MotionControl.cpp //also interesting, they use the RMT instead of timers, quite possible to get higher step rates that way I think
Depending on the goal of this library, this might be out of scope.
Regarding Linear Axis classes inheriting base single axis movement classes, that seems reasonable to me, and can easily be wrapped by a user's planning strategy if it can support some kind of plan-ahead thing.
eldendiss
commented
11 months ago I've seen problems in other libraries with that method Bresenham's algorithm in the planner for each ISR iteration, and emitting a pulse according to the generated coordinates for that axis. It would generates unequally spaced steps if one of the axis were not moving at even pulse intervals. There's a large chance a stepper driver will miss a step if the instantaneous pulse width calculation generates a pulse that is too narrow or too close to the next step, when coordinating two axis moving together at the same time unless a closed loop stepper was used.
Yes I suspect that this happens even in the current version, with higher steps/s. (I personally blame cache miss for this, but I haven't done the tests). It would be interesting to see ISR execution time if the algo is optimized to use fixed point math and all gptimer functions are placed in IRAM along with the ISR function. That would however have other consequences which may or may not matter in final application.
also interesting, they use the RMT instead of timers, quite possible to get higher step rates that way I think
When I started developing the library, I've also tried the RMT approach, however there have been some problems with weird pulses or jerks manifesting when following ramp profile. Maybe I2S could be also used. Also the RMT API changed significantly and I can see some added functionality, so maybe it would be worth trying out again.
I think that Axis classes could inherit the base class, as pointed out in your feedback.
cedricwritescode
commented
11 months ago I agree that there could be issues concerning the ISR. Maybe my use case really shouldn't be part of this library, as I'm just trying to build a cnc machine for fun (and could just use FluidNC). There is an official esp-idf example using RMT for stepper motor control. Nonetheless, I think the movement planner would benefit from some sort of modular design.
eldendiss
commented
9 months ago A little update, the new API is in progress. Currently I was evaluating using RMT for step signal generation, which would be ideal for motion planner, however it seems that RMT loop feature is needed in order for this to work smoothly, and sadly its not supported by earlier ESP32 chips.
The loop transmit feature is not supported on all ESP chips, please refer to [TRM] before you configure this option, or you might encounter ESP_ERR_NOT_SUPPORTED error.
Therefore I will stick to existing ISR approach.
MR-VENOM
commented
9 months ago Therefore I will stick to existing ISR approach.
@eldendiss is it possible to implement an option to choose between RMT and ISR if using with esp32s3 ? or if i can get the rmt method you tried it would be nice i am planning for a s-curve profile . with RMT i can queue my motions until the previous is done
Is your feature request related to a problem? Please describe. I find the structure of the library a bit unintuitive. Therefore I propose these changes:
Describe the solution you'd like
Separate linear movement functionality from actual motor control into different classes The linear movement class could inherit from the actual stepper class.
Change properties of motor and motors connected to linear axes Change microStepping and stepAngle to stepsPerRot. This would give the user more freedom to decide on microstepping values, as well as being a more descriptive property. The user could set it like (motor_steps_per_turn * controller_microsteps). Additionally, I think stepsPerMm mixes properties of linear and stepper functionality. I propose to use feedConstant instead (i.e. mm/turn).
Simplify initialization (i.e. init, init(params), config methods) Generally, there should be a single method the user calls for initialization. Right now there are 3 methods, which uses might be a bit ambigous. I think the best way would be to only use the init struct, as it is more concise and adaptable than passing every config value as a method parameter. I think performing initialization in the constructor is not optimal. The reason is that a possible error during a c'tor call would be less debuggable than a runtime error.
Describe alternatives you've considered
Additional context