Perform preliminary firmware tests on the AMC-BLDC

[mfussi66]

Task description 📝

We recently refactored the motor control supervisor (see #65). We now have to test the new architecture on the AMC-BLDC board.
For this we need:

• to restructure the AMC-BLDC architectural model with the new supervisor
• a setup with a motor

Useful links and documents

Definition of Done ✅

Demo with setup running with the new firmware.

---

Originally opened by @fiorisi in different repo.

---

As suggested by @pattacini, it will be useful to create the AMCBLDC customizations in a separate path, so that the two versions can live side by side.

[mfussi66]

The architectural model could become the following:

• The supervision is split between motion controller (RX, new version) and base model (TX, coming from amcbldc)
• The messaging remains untouched
• The Motion Controller is instantiated for one motor, and includes
  • FOC
  • Outer control
  • Supervisor RX (new version)
  • Estimation

Some considerations:

• One dictionary will win over the other, will it be common.sldd or the original AMCBLDC one called EmbeddedBoard.sldd? I suggest drawing from common
• errors_rx and status_rx are unused at the moment, maybe it's useful to restore their usage in the new supervisor
• The enums in the motion controller will need to be checked and aligned with the AMCBLDC
• A global Configuration struct needs to be passed from outside
• The interface adapter needs to be updated

[pattacini]

All good 👍🏻

A couple of comments:

• A global Configuration struct needs to be passed from outside

We should keep adhering to the way we did it on AMCBLDC: a Simulink parameter defined in the dictionary that is used to initialize the internal structures, thus NO dedicated inputs to the architecture.

• One dictionary will win over the other, will it be common.sldd or the original AMCBLDC one called EmbeddedBoard.sldd? I suggest drawing from common

Definitely common.sldd 👍🏻
Switching back to EmbeddedBoard.sldd? makes no sense!

Warning

Be careful as we shall still deal with the CAN-based part of the dictionaries.

At the moment, this is the dictionary hierarchy:

Important

The dictionary can_messaging.sldd needs to be preserved and grafted into common.sldd as a child, somehow.

Tip

Actually, the name common.sldd doesn't sound great 😄
Any other options?

[mfussi66]

We should keep adhering to the way we did it on AMCBLDC: a Simulink parameter defined in the dictionary that is used to initialize the internal structures, thus NO dedicated inputs to the architecture.

Good point, the GlobalConfiguration includes estimation velocity mode, the lambda for rms computation and so on, not really configurable stuff.

The dictionary can_messaging.sldd needs to be preserved and grafted into common.sldd as a child, somehow.

I suggest this dictionary structure:

amcbldc.sldd inherits from the other three, and can contain customizations for the board.
Btw, I am not so sure that supervisor.sldd is needed anymore.

Important

The supervisor at the moment cannot support 4 messages (or more generally, events) per tick, but only one.

[pattacini]

amcbldc.sldd inherits from the other three, and can contain customizations for the board.

Oki 👍🏻

Btw, I am not so sure that supervisor.sldd is needed anymore.

Indeed, let's get rid of it 👍🏻

The supervisor at the moment cannot support 4 messages (or more generally, events) per tick, but only one.

This appears to be a severe limitation.
We should discuss it.

[mfussi66]

This appears to be a severe limitation.
We should discuss it.

Taking inspiration from the implementation in the original supervisor, I readded the multiple processing of CAN messages (called generically Received Events) in the following fashion.

The inputsDispatcher was changed in such a way that every time it is called, it iterates over the MAX_NUMBER_EVENTS array of events, and with a switch-case pattern (suggested by matlab) it runs the appropriate functions and stateflow events

The generated code is the following, which to me seems simple and effective enough:

    /*  Consume N events per tick  */
    for (ei = 0; ei < MAX_EVENTS_PER_TICK; ei++) {
      switch (supervisor_U->ReceivedEvents[ei].event_type) {
       case SupervisorEvents_SetLimit:
        supervisor_SetLimits(supervisor_U->ReceivedEvents[ei].
                             limits_content.overload,
                             supervisor_U->ReceivedEvents[ei].
                             limits_content.peak, supervisor_U->
                             ReceivedEvents[ei].limits_content.nominal,
                             supervisor_U, supervisor_Y, supervisor_DW);
        break;

       case SupervisorEvents_SetPid:
        supervisor_SetPid(supervisor_U->ReceivedEvents[ei].pid_content.type,
                          supervisor_U->ReceivedEvents[ei].pid_content.P,
                          supervisor_U->ReceivedEvents[ei].pid_content.I,
                          supervisor_U->ReceivedEvents[ei].pid_content.D,
                          supervisor_U->ReceivedEvents[ei].
                          pid_content.shift_factor, supervisor_Y);
        break;

       case SupervisorEvents_SetMotorConfig:
        supervisor_Y->ConfigurationParameters.motor =
          supervisor_U->ReceivedEvents[ei].motor_config_content;
        break;

       case SupervisorEvents_SetTarget:
        supervisor_SetTarget(supervisor_U->ReceivedEvents[ei].
                             targets_content.jointvelocities,
                             supervisor_U->ReceivedEvents[ei].
                             targets_content.motorcurrent,
                             supervisor_U->ReceivedEvents[ei].
                             targets_content.motorvoltage, supervisor_Y,
                             supervisor_DW);
        break;

       case SupervisorEvents_SetControlMode:
        supervisor_DW->requiredControlMode = supervisor_U->ReceivedEvents[ei].
          control_mode_content;
        b_previousEvent = supervisor_DW->sfEvent;
        supervisor_DW->sfEvent = supervisor_event_SetCtrlMode;
        if (supervisor_DW->is_active_ControlModeHandler != 0U) {
          supervisor_ControlModeHandler(supervisor_U, supervisor_Y,
            supervisor_DW);
        }

        supervisor_DW->sfEvent = b_previousEvent;
        break;
      }
    }

Note that if a ReceivedEvent is not recognized in type, it is simply skipped.

This part might need to be tested before being integrated into the AMCBLDC architectural model.

[pattacini]

Nice!

[mfussi66]

I created a test sequence in which I send all the "preparation" messages in a tick:

1. limits
2. pid cfg
3. motor cfg
4. control mode set

and on the same tick, the control mode was set correctly with the rest of the parameters:

[pattacini]

Super!

[mfussi66]

What is needed to complete the integration is now the interfacing of the CAN Decoder and the Motion controller.
Most specifically, it is necessary to adapt the data structs containing the decoded can messages, and turn them in what i called "supervisor received events". That can be done with an additional simulink block, interposed between the two, or by adapting the CAN decoder to the new proposed schema.

[pattacini]

That can be done with an additional simulink block, interposed between the two, or by adapting the CAN decoder to the new proposed schema.

We're working in a fully parallel "branch" of development (actually, it's a dedicated folder) meaning that you're dealing with a copy of the current AMCBLDC structures. Hence, it should be safe to adapt the CAN Decoder without impacting what is running now on the boards.

[mfussi66]

Today I was able to adapt the CAN decoder so that it could be attached to the new motion controller, and successfully generate the cod, both in C++ 03 and C99 . The signal status_rx is not needed anymore since for each of the max 4 can messages, we now have a enum that specifies that kind of data it is. After it is consumed, it's only needed to reset it to SupervisorEvents.None.

[mfussi66]

The following commit in my fork allows compilation of the new version of the AMCBLDC: mfussi66/icub-firmware@b0cc008 .

Steps I performed:

• Change codegen option to Nonreusable function so that the step() functions did not need to accept arguments
• Add newly created codegen files to keil project
• Adapt glue code so that the updated struct members would be called (it's only name changes)

[mfussi66]

A very first test seems promising! Here on the right I am printing the control mode after each can message sent, by reading the Flags in the glue code:

For some reason I do not see the FOC and Status messages on the CAN, I will check.

[mfussi66]

Now the CAN messages are sent, because I forgot to set the flag enable_sending_msg_status:

However, now they are always sent, I think because there is no reset for the message type (setting it to supervisorevents.none) after it is consumed.

[pattacini]

However, now they are always sent, I think because there is no reset for the message type (setting it to supervisorevents.none) after it is consumed.

On the old architecture, I think we have something very similar, although I actually don't remember how we did it.
It may be worth checking it out.

[mfussi66]

I think I figured out a way that is a bit more simple and I think elegant: The CAN decoder stateflow chart has an option called "Initialize outputs every time the chart wakes up": if that is enabled, at the beginning of each Ts the outputs are reset to the initial values, which I think is a sensible option for the decoder.

In C code, this means that the output variables are reinitialized every time can_decoder() is called, in the beginning (hopefully it does not cause an important overhead).

But now it works as expected:

amcbldc.mp4

[pattacini]

Super! Very elegant, indeed.

Perhaps, it could be beneficial to drop a comment somehow in one FSM to bring up this important mechanism of initialization, especially for the newcomers.

[mfussi66]

Perhaps, it could be beneficial to drop a comment somehow in one FSM to bring up this important mechanism of initialization, especially for the newcomers.

Done that.

After connecting the motor, it does not spin, because the current does not attempt to reach the setpoint. Debugging it at the moment.

[mfussi66]

We now have the working current controller, but the motor does not spin:

the current does not attempt to reach the setpoint

This was caused by the CAN decoder that would send only a partially set MotorConfiguration struct. Therefore, Vcc and Vmax were = 0.

I solved it by adding the following line on top of the can decoding of the motor config message:

[pattacini]

Aligned F2F with @mfussi66:

• We need to differentiate the structure used to contain the conf data (that gets initialized via InitConf) and the structure used to handle CAN messages.

[mfussi66]

• We need to differentiate the structure used to contain the conf data (that gets initialized via InitConf) and the structure used to handle CAN messages.

Yes, I wrapped the elements that can be configured via CAN in a struct called MotorConfigurationExternals, which is then included in the global MotorConfiguration.

And we now have a spinning motor!

I just need to understand where to put the hall_sensors_offset parameter, but I think it can be put in the initial configuration, editable via user-facing script.

[mfussi66]

I also fixed a mistake (caused by me) that caused the motor not to be enabled after fault button, now the sequence:

1. fault button pressed
2. fault button released
3. control mode set
4. target set

is correctly handled, just as before.

However, setting the FOC Ts to 45us makes the motor spin with more oscillations, is this expected?

[pattacini]

However, setting the FOC Ts to 45us makes the motor spin with more oscillations, is this expected?

Hi @sgiraz

I don't seem to find the test results we did while checking the performance of the amcbldc running with T=45 us. Can you please have a look?

It's important to verify the quality of control.

[pattacini]

However, setting the FOC Ts to 45us makes the motor spin with more oscillations, is this expected?

Hi @mfussi66

Did you check that the HAL is configured to have the correct PWM rate as well?

[mfussi66]

However, setting the FOC Ts to 45us makes the motor spin with more oscillations, is this expected?

Hi @mfussi66

Did you check that the HAL is configured to have the correct PWM rate as well?

Yes, I was rebased on the latest devel which had the changes in the hal and its commits.

I must note that this behaviour seems to be happening also with the devel. Maybe it's just a matter of tuning up the current pid? They have never been touched, even between different motor models.

[pattacini]

Ok, I thought you noticed the behavior while switching to Ts = 45 us, whereas it wasn't like that with the previous Ts.

It's true that we can tune the PID gains, but our understanding is that changing Ts doesn't cause any macroscopic change in the logged quantities (e.g., current, position, velocity), keeping conditions (e.g., motor, setup, board) unvaried.

Can you confirm that, @mfussi66, with your current setup?

[mfussi66]

Ok, I thought you noticed the behavior while switching to Ts = 45 us, whereas it wasn't like that with the previous Ts.

That's what I mean, both devel and my refactor, with the new Ts, exhibit more oscillations in Current control mode.

I can verify that by trying both devel and refactor with Ts = 45 us and Ts = 26.6 us 37.5us, in a separate issue.

[mfussi66]

When inspecting the control mode switches to finalize the tests on the board, I noticed three occasions that I think are worth attention.

See the plot below:

I put arrows on the three events:

• Switch from Voltage mode to current: The measured Iq, when put as setpoint as bumpless transition, becomes higher than the measurement

• Changing from any mode to velocity sets the velocity setpoint to zero: I think this is expected, since it was like that also in the previous architectural model version

• Switch from Current mode to Voltage: In this case, the Vq that is imposed in voltage mode is significantly lower than the Vq that is measured in Current mode: this is most likely caused by this:

The Vq sent as feedback is scaled by Vcc. But if the same value is set as reference (such as a bumpless change of control mode), the value is scaled by Vmax. If Vmax < Vcc , the applied voltage lowers.

[mfussi66]

Changing from any mode to velocity sets the velocity setpoint to zero: I think this is expected, since it was like that also in the previous architectural model version

Working as expected ✔

Switch from Current mode to Voltage: In this case, the Vq that is imposed in voltage mode is significantly lower than the Vq that is measured in Current mode: this is most likely caused by this:

Targets.voltage needs to be scaled by Vcc and not Vmax.

[mfussi66]

And now it works:

The current looks much worse of course, since it's open loop control. but the Vq is the same.

[pattacini]

Fixed master via 1be8d97 and then merged into devel.

Here's the fix:

cc @mfussi66

[mfussi66]

Switch from Voltage mode to current: The measured Iq, when put as setpoint as bumpless transition, becomes higher than the measurement

This is the last point to address: I'll also check if it happens in the original version of the architectural model.

[mfussi66]

Indeed, it seems that it happens also with the upstream architectural model, see the video.

Given that this workflow is very unlikely to happen, it might be useful to notify it in a separate issue with low priority.

MATLABWindow_hnlp1F7PTL.mp4

[pattacini]

Given that this workflow is very unlikely to happen, it might be useful to notify it in a separate issue with low priority.

Agree 👍🏻

[pattacini]

Follow-up issue created:

• The switch Voltage → Current control is NOT bumpless #86

I think you can consider this task completed, @mfussi66 🎉

[mfussi66]

Ok so I think a recap of the must have features is in order:

• Ability to use 3 different control modes: Direct, Current, Velocity (and idle)
• Handling of fault button and mode restore
• 4 can messages handled per tick
• Bumpless transition between modes

If no feature is missing, I think we can indeed complete the task.

[mfussi66]

As requested, I also tested succesfully if the overcurrent flag was currently set. Here the overload limit is set to 50mA and the target is 100mA.

[mfussi66]

To conclude this issue, I created these two PRs:

• Common project is updated with features and files from amcbldc v1 #87
• Add Experimental AMCBLDC project #88