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

Improvements to LinearHall #39

Merged
merged 3 commits into from
Feb 14, 2024
Merged

Improvements to LinearHall #39

merged 3 commits into from
Feb 14, 2024

Conversation

dekutree64
Copy link
Contributor

Changes compared to the original pull request #12

  1. Added a version of init which turns the motor one revolution to find the center values of the sensors.
  2. Moved the calls to analogRead into a weakly bound function ReadLinearHalls so it can be overridden with custom ADC code on platforms with poor analogRead performance.
  3. Commented out the pinMode calls in init, which makes it possible to pass in ADC channel numbers for custom ReadLinearHalls to use without having to remap them every update.
  4. Changed to use the much faster _atan2 function that was added to foc_utils recently.
  5. Added example

Changes compared to the original pull request simplefoc#12

1. Added a version of init which turns the motor one revolution to find the center values of the sensors.
2. Moved the calls to analogRead into a weakly bound function ReadLinearHalls so it can be overridden with custom ADC code on platforms with poor analogRead performance.
3. Commented out the pinMode calls in init, which makes it possible to pass in ADC channel numbers for custom ReadLinearHalls to use without having to remap them every update.
4. Changed to use the much faster _atan2 function that was added to foc_utils recently.
@runger1101001
Copy link
Member

Thank you very much for this PR!

Is it ready for merging? I would be happy to merge it.

There is a compile problem with the example for ESP32s - they don't define pin A1 it seems - many ESP32 boards don't use the Arduino pin conventions.

I would change the example to use numeric pin numbers, or else we can also just remove the ESP32 compile from the test cases, whichever you think is better...

@dekutree64
Copy link
Contributor Author

Yes, it's good.

Any solution for the compile error would be fine with me, including scrapping the example entirely. I thought the A0, A1, etc. pin names were part of the Arduino library spec, but if not then it would be better to use numeric pins. It may be a while before I get around to doing it (I have to boot up Windows 10 to use github desktop), so feel free to do it yourself if you'd like to get it merged sooner.

@runger1101001
Copy link
Member

Perfect, lets merge it. Since the example compiles on all the other test architectures, I'll just exclude ESP32.

@runger1101001 runger1101001 changed the base branch from master to dev February 14, 2024 21:01
@runger1101001 runger1101001 merged commit 5e9981c into simplefoc:dev Feb 14, 2024
9 of 10 checks passed
@runger1101001 runger1101001 added this to the 1.0.7 milestone Mar 29, 2024
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