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

AMCBLDC: Fix computation of angle when using quadrature encoder #332

Merged
merged 3 commits into from
Jan 13, 2023

Conversation

mfussi66
Copy link
Member

@mfussi66 mfussi66 commented Jan 11, 2023

This PR is aimed at the AMCBLDC Hal, to fix:

Detection of the sector of the phases

line 231 in pwm.c

In this line we compute the sector that the electric angle belongs to = [0, 1, ..., 5].
Casting to int16 would cause sector to have the values [-5 -3 -1 1 3 5], which would mess up the later computations

Desynching between hall sensors angle and quadrature encoder angle

A reset function of the quadrature encoder angle is applied whenever a full rotation is done.
A full rotation is defined as the transition between sector 5 and sector 0.
This procedure is also used to during calibration step 0, to transition to step 1:

  • Step 0: Rotate using the Hall sensors until there's the switch between sector 5 and 0, reset the encoder, and go to step 1
  • Step 1: Rotate using the Hall sensors until we read the uncalibrated encoder value for each of the 6 sectors (or borders);
    Then, we compute the offset by taking the average of the 6 values, and apply it.
  • Step 2: Store the electric angle evaluated by the hall sensors as always, and reset the encoder if a full rotation is done

cc @sgiraz @pattacini @valegagge

@mfussi66 mfussi66 self-assigned this Jan 11, 2023
Copy link
Contributor

@marcoaccame marcoaccame left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mfussi66, the change is fine. I assume that it was tested on a dedicated setup and everything works.

It is important however also to explain how are obtained the magic numbers used in pwm.c.

It may be enough a brief description inside the PR which points to an inline comment in the code w/ more details.

Later on (a further PR is fine) we shall collect all the comments inside a motorhal/docs/readme.md file which documents the internals of the motorhal.

@mfussi66 mfussi66 marked this pull request as draft January 12, 2023 13:00
@mfussi66
Copy link
Member Author

@marcoaccame I added comments to the code and some constexpr to give meaning to the hardcoded numbers, let me know if they add clarity to the code.
I'll keep testing the motor to make sure it all works as expected before marking the pr ready for review.

@mfussi66 mfussi66 marked this pull request as ready for review January 12, 2023 17:44
Copy link
Contributor

@marcoaccame marcoaccame left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, thanks

@marcoaccame marcoaccame merged commit 39fe0ca into robotology:devel Jan 13, 2023
valegagge pushed a commit to valegagge/icub-firmware that referenced this pull request May 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants