SRA-VJTI / sra-board-component

ESP-IDF component for SRA Board
https://sravjti.in/sra-board-component/
MIT License
36 stars 20 forks source link

Motor Driver Initialisation #14

Closed laukik-hase closed 4 years ago

laukik-hase commented 4 years ago

https://github.com/SRA-VJTI/sra-board-component/blob/52923c8263789b255aa6c56a5a1de9f0cb9ad68b/src/motor_driver.c#L7

@VedantParanjape @luke2103 @OSSome01 Rather than having two different functions (with the same template) for the two drivers, can we have a single function which takes the motor driver to be initialised as parameter?

VedantParanjape commented 4 years ago

Earlier it was done similarly, but it increased the complexity and made the function large and difficult to read.

Maybe I can just use macros to do this task ?

VedantParanjape commented 4 years ago

#define enable_motor_driver(id, mode) enable_motor_driver_##id(mode)

something like this

VedantParanjape commented 4 years ago

@laukik-hase

laukik-hase commented 4 years ago

#define enable_motor_driver(id, mode) enable_motor_driver_##id(mode)

something like this

@VedantParanjape This could work. But, how can we make the code compact as there will be different initialization parameters for each of the function call? (E.g. MDA_PARALLEL_IN_3_4)

VedantParanjape commented 4 years ago

@laukik-hase I don't think we need to pass MDA_PARALLEL to enable_motor_driver function. Call will be something like

enable_motor_driver(a, PARALLEL);
enable_motor_driver(b, NORMAL);
laukik-hase commented 4 years ago

@VedantParanjape Cool then, carry on with it.

VedantParanjape commented 4 years ago

@laukik-hase How should I go about motor_forward function ? currently it takes mcpwm timer id as parameter which is very confusing even to me.

Should follow same pattern of a/b used in enable function.

laukik-hase commented 4 years ago

@VedantParanjape Yes. Just be a bit careful with the MCPWM Units and Timers for all the motors.

VedantParanjape commented 4 years ago

@laukik-hase Closing this issue, please create a new issue for the fresh review.