Skip to content

Driver refactor #153

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

Merged
merged 15 commits into from
Feb 12, 2022
Merged

Driver refactor #153

merged 15 commits into from
Feb 12, 2022

Conversation

runger1101001
Copy link
Member

Refactoring of all the PWM drivers to return a parameters stucture pointer of void* type on initialisation (if successful).

The same pointer is passed to the _writeDutyCycle() methods instead of passing all the pins each time.

  1. This allows each driver to store its internal state, reducing the need for static variables and making multi-motor implementations easier. It also allows the drivers to potentially store other references than the pins, enabling fewer lookups and thus faster execution when setting PWM.
  2. It will allow future current-sensing implementations to obtain information from the driver. Since low-side sensing in particular has to be synced to the PWM this is a necessary pre-condition to doing low-side sensing.

At the moment this is in status "in development", but since there is no easy way to do this "driver by driver", I wanted to switch them all over early in this release cycle to give us plenty of time to test the changes, which affect every architecture.

So far I have tried it out on:

  • SAMD21 - motor spinning
  • SAMD51 - motor spinning
  • NRF52 - motor spinning
  • STM32F405 - checked PWM only

This change is ready for testing, but there will be further changes as we work with it:

  • I have changed only the driver API, and where obvious have stored the required references for that MCU architecture. When we next make changes in a given architecture we may still further optimize what is stored in the driver params for that architecture.
  • When we implement current sensing, we will have to move the definitions of the different driver parameter structs into header files so they can be shared between current sense and driver code. I did't do this yet for all the architectures.

But these changes should not further affect the outward facing API of the drivers.

I think we should merge it into dev now, and then we can properly start on low side current sensing... as we implement this we will try out all the architectures...

@askuric
Copy link
Member

askuric commented Feb 12, 2022

Its a great mod and we will test it in depth in dev branch. From what I was able to see in the code your changed definitely make sense.
This is a very good base for current sensing support.

@askuric askuric merged commit 3704a14 into simplefoc:dev Feb 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants