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

Run a test campaign before merging AMCBLDC_experimental and AMCBLDC #92

Closed
mfussi66 opened this issue Jun 6, 2024 · 20 comments · Fixed by #94
Closed

Run a test campaign before merging AMCBLDC_experimental and AMCBLDC #92

mfussi66 opened this issue Jun 6, 2024 · 20 comments · Fixed by #94
Assignees
Labels
domain-firmware Pertains to FW development team-dev Activity performed by Team Dev

Comments

@mfussi66
Copy link
Member

mfussi66 commented Jun 6, 2024

At the moment there are two projects that involve the AMCBLDC board, the official one and the Experimental one.
The Experimental project involves a major refactor of the supervisor and leverages the Common Simulink project to make the structure more modular.

While the first trials were successful (#84), the integration tests were not exhaustive.

This issue aims to track which tests need to be performed, before making Experimental the official project.

@mfussi66 mfussi66 added domain-firmware Pertains to FW development team-dev Activity performed by Team Dev labels Jun 6, 2024
@mfussi66 mfussi66 self-assigned this Jun 6, 2024
@mfussi66
Copy link
Member Author

mfussi66 commented Aug 22, 2024

I was able to successfully restore operation of the setup, so I can lay out the integration tests:

Must:

  • Switch control mode any-to-any:
    • Idle -> current ✅
    • Idle -> velocity ✅
    • idle -> voltage ✅
    • current -> idle ✅
    • current -> velocity ✅
    • current -> voltage ✅
    • voltage -> idle ✅
    • voltage -> current ✅
    • voltage -> velocity ✅
    • velocity -> idle ✅
    • velocity -> current ✅
    • velocity -> voltage ✅
    • The switch between modes must be bumpless
  • Restoring the control mode from idle/fault should reset the setpoint ✅
  • Velocity mode must be succesfully tested ✅
  • Open loop voltage mode must be succesfully tested ✅

Could:

  • If possible, depending on the Simulink driver, 4 CAN messages need to be processed in the same tick ✅

This is a first list, other tests will be added as needed.

@mfussi66
Copy link
Member Author

mfussi66 commented Aug 23, 2024

Could:

* If possible, depending on the Simulink driver, 4 CAN messages need to be processed in the same tick

The Simulink CAN driver is indeed able to send multiple messages in a Simulation tick. The vectorized messages though are not really parallel, but sequential and they are sent every 100us.

The board is indeed able to get them, and we can see that from a cold start it processes all the 4 messages since it switches to a control mode and sends its own messages.

image

@mfussi66
Copy link
Member Author

mfussi66 commented Aug 23, 2024

We can see from the plot below that the bumpless transfer works also for the velocity mode, though since the PIDS are reset, the transitory spike is present:

bumpless

This can be improved by updating the initial conditions of the integrators, and maybe tuning up a little bit the controllers.

@pattacini
Copy link
Member

pattacini commented Aug 24, 2024

Well done so far 👍🏻

This can be improved by updating the initial conditions of the integrators, and maybe tuning up a little bit the controllers.

We do employ Tracking Mode in our PID controllers, hence the bumpless switch should be guaranteed. Perhaps, we could check whether the tracking coefficient (Kt) is ok. See:

@pattacini
Copy link
Member

From the Astrom's bible, it turns out that we can increase Kt in order to have more bumpless switches.

image

@mfussi66
Copy link
Member Author

mfussi66 commented Aug 26, 2024

A first try of increasing the tracking coefficient by a factor of 100 seems to suggest that the PID parameters still dominate the response,

Note that the setpoint is kept at 50mA here.

untitled

I might make more tests in simulation nonetheless.

@mfussi66
Copy link
Member Author

Regarding

Restoring the control mode from idle/fault should reset the setpoint

All tests are successsful:

Voltage

voltage

Current

cur

Velocity

vel

@mfussi66
Copy link
Member Author

I might make more tests in simulation nonetheless.

It seems that even in simulation the effect of the tracking coefficient Kt is irrelevant for the Iq controller, see the comparison below.

In this scenario, I increased 10x the current PID:

untitled

There is a 3msec difference in the settling time to 99.5% of the target.

Note that this behaviour occurs with the current AMCBLDC architectural model as well.

Maybe we can track the developments on this front in a different issue.

@pattacini
Copy link
Member

Maybe we can track the developments on this front in a different issue.

Yes... and no 😃
Meaning I don't think it's worth spending too much time on this after having documented how it works so in detail.

The issue can be opened, but I'd be more in favor of continuing with the final switch.

@mfussi66
Copy link
Member Author

mfussi66 commented Aug 30, 2024

I'm ready to create the PR to replace the current AMCBLDC architectural model, and in icub-firmware, but I found issues with the CMSIS, after upgrading to Matlab R2024a:

  • The files mw_cmsis.h and mw_cmsis_impl.h (matlab headers wrapping the cmsis functions of arm_math.h) would not be copied from the internal matlab folders to the codegen ones
  • The CMSIS function called by this version of matlab 2024a are the 5.9, not the latest version (6.1). Therefore, we would need to pin on the keil project a specific cmsis and cmsis-dsp packages.
  • Even after pinning the version of the cmsis libraries and copying manually the files, I'm still having issues compiling the code, because it would not find cmsis functions.

I suggest for the time being to completely avoid calling cmsis functions even for this experimental version, since we saw no benefit in the critical paths of the FOC in the past.

See :

@pattacini
Copy link
Member

Do you have the latest MATLAB updates installed?

Let's go w/o CMSIS 👍🏻
However, I would really recommend filing a ticket on MathWorks about this bug.

@mfussi66
Copy link
Member Author

Do you have the latest MATLAB updates installed?

Yes, Update 6.

Let's go w/o CMSIS 👍🏻 However, I would really recommend filing a ticket on MathWorks about this bug.

Will do 👍🏻

@pattacini pattacini linked a pull request Aug 30, 2024 that will close this issue
@mfussi66
Copy link
Member Author

PR here for icub-firmware-models: #94

Will request merging after successful tests on the wrist.

@mfussi66
Copy link
Member Author

mfussi66 commented Sep 4, 2024

Booked the robot ergocub 000 for tomorrow.

@mfussi66
Copy link
Member Author

mfussi66 commented Sep 5, 2024

The integration test on the robot could involve the following operations:

Test Result (❌ or ✅)
Put the boards in Current mode at startup
Move the motors by applying a Yaw setpoint
Move the motors by applying a Pitch setpoint
Move the motors by applying a Roll setpoint
Push the fault button, release it, restore control mode -
Set a small overcurrent limit, trigger overcurrent fault, clear it, restore control

Important

Other control modes are not available because of the joint coupling

cc @fiorisi

@mfussi66
Copy link
Member Author

mfussi66 commented Sep 5, 2024

These are the first results coming from the robot.

I created a set of configuration files to launch only the left wrist joints, and dumped the data with yarpdatadumper, monitoring the port /wrist/mc/stateExt:o, so I could get the current and pwm info from the stateExt struct.

The tests were successful, the only one I could not run was the fault button press and release, because there is an issue with the robot. However, the fault button test works on the lego steup.

I called the firmware 2.1

Here are the results of position control for all 3 joints, they work as expected:
position

Here are the resulting pwms (perc [0, 1]) and currents (Ampere). The values are quite chaotic, but I do not think they are caused by the refactored firmware.
currents_pwm

Here is the result of the overcurrent test and the control mode switch, in which the behaviour is expected>

position -> fault -> idle > position

The control mode integer values are the result of the vocab32 creation:

mode vocab integer
position pos 7.56e6
idle idl 7.10e6
hardware fault hwfa 1.63e8

modes

@pattacini
Copy link
Member

Fantastic job @mfussi66, really 🚀
I will merge the PR.

These changes won't be delivered via distro 2024.08.0 though. They'll be available in devel.

Therefore, we ought to make sure that the corresponding generated source FW along with binaries won't land in the release.

cc @Nicogene

@mfussi66
Copy link
Member Author

mfussi66 commented Sep 5, 2024

Therefore, we ought to make sure that the corresponding generated source FW along with binaries won't land in the release.

Will do 👍
Though I must note that this new mbd firmware was not tested on the AMC board. We can schedule it with another issue.

@pattacini
Copy link
Member

Though I must note that this new mbd firmware was not tested on the AMC board. We can schedule it with another issue.

Agree 👍🏻
Go ahead and open up an issue for that.

I'll merge the PR anyway as we have a copy of the old code.

@pattacini
Copy link
Member

PR merged ✅

@mfussi66 mfussi66 changed the title Run a test campaign before merging AMCBLDC_experimental and AMCLBDC Run a test campaign before merging AMCBLDC_experimental and AMCBLDC Sep 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain-firmware Pertains to FW development team-dev Activity performed by Team Dev
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants