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

Reset quadrature encoder counter via HW #455

Merged
merged 4 commits into from
Jan 12, 2024
Merged

Conversation

ale-git
Copy link
Contributor

@ale-git ale-git commented Dec 28, 2023

This PR solves a synchronisation problem in the quadrature encoder conter reset by index.
In the previous version the conter was reset by a interrupt service routine activated by the index, affected by response delay, while now the conter is reset by hardware.

@pattacini pattacini changed the title Devel Reset quadrature encoder counter via HW Dec 28, 2023
@valegagge valegagge self-requested a review January 8, 2024 08:26
@valegagge
Copy link
Member

please see here for more details about the diagnostic.

@valegagge
Copy link
Member

We already did some tests using this new feature and we obtained very good results.
#446

We'll put the PR in ready after we addressed #452

@valegagge valegagge requested a review from marcoaccame January 8, 2024 10:43
@valegagge
Copy link
Member

Hi @ale-git ,
just one thought: I noticed that now in the case of encoder-dirty or index-broken you call the QE_RISE_ERROR macro that calls the fault handler. The last sets the motor at fault.

#define QE_RISE_ERROR(e) gEncoderError.e=TRUE; SysError.EncoderFault=TRUE; FaultConditionsHandler()

image

Is this correct?

If yes, in this way we are much more strict than before, when the index broken and the encoder-dirty are only warnings.
Are we sure we want to be so severe?

@ale-git
Copy link
Contributor Author

ale-git commented Jan 10, 2024

Hi @ale-git , just one thought: I noticed that now in the case of encoder-dirty or index-broken you call the QE_RISE_ERROR macro that calls the fault handler. The last sets the motor at fault.

#define QE_RISE_ERROR(e) gEncoderError.e=TRUE; SysError.EncoderFault=TRUE; FaultConditionsHandler()

image

Is this correct?

If yes, in this way we are much more strict than before, when the index broken and the encoder-dirty are only warnings. Are we sure we want to be so severe?

I think that if the diagnostics is reliable they should be errors and not warnings, at least the index broken. We can make the encoder dirty just a warning, but people should be forced to clean encoders when they are dirty because they spoil the performance of the control.

@valegagge
Copy link
Member

Hi @ale-git , just one thought: I noticed that now in the case of encoder-dirty or index-broken you call the QE_RISE_ERROR macro that calls the fault handler. The last sets the motor at fault.
#define QE_RISE_ERROR(e) gEncoderError.e=TRUE; SysError.EncoderFault=TRUE; FaultConditionsHandler()
image
Is this correct?
If yes, in this way we are much more strict than before, when the index broken and the encoder-dirty are only warnings. Are we sure we want to be so severe?

I think that if the diagnostics is reliable they should be errors and not warnings, at least the index broken. We can make the encoder dirty just a warning, but people should be forced to clean encoders when they are dirty because they spoil the performance of the control.

After a discussion with @ale-git e @MSECode,
we decided to set the index-broken as a warning even if we agree with @ale-git that it is an error and not a simple warning, but for now, we want to avoid the robot falling while it is walking or doing other activities.

We planned to define the index-broken as critical notifying the user of the problem and letting him/her find a "safe" position as soon as possible because an index broken means that the controller doesn't work properly.

@ale-git ale-git self-assigned this Jan 11, 2024
@ale-git ale-git marked this pull request as ready for review January 11, 2024 22:47
@ale-git
Copy link
Contributor Author

ale-git commented Jan 11, 2024

Tested on ErgoCub2 arm.

dirty and index_broken encoder faults are managed as warnings. In case of index_broken the warning is generated and the counter is resetted by software matching it to QE_RESOLUTION until the synchronicity is lost. In case of dirty the motor continues to run compatibly with reduced performance.

Executable here robotology/icub-firmware-build#128

Copy link
Member

@valegagge valegagge left a comment

Choose a reason for hiding this comment

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

well done!!! 🚀

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

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.

3 participants