Voltage limiting missing for some cases (in motion control velocity, angle)

[greymfm]

voltage limiting is missing (for both velocity motion control and angle motion control) for the case if no phase_resistance is given:

https://github.com/simplefoc/Arduino-FOC/blob/e9ac395cb9d698c0aa6d779206a6c413fbedcbb3/src/BLDCMotor.cpp#L416

The corrected (and tested code) looks like this:

[askuric]

Hey @greymfm, I'm happy to see that you're duigging deep in the libray code, however the voltage limiting is present in velocity and angle control using the PID_velocity. It has in integrated output limiting and it is set to the voltage_limit.

Regarding the torque control using voltage, limiting was intentionally left out in the library versions so far. However this has changed recently and the next release will have this limiting as well. So when you're in voltage mode, you'll not be able to set the target higher than voltage_limit.

[runger1101001]

In this case the output is already constrained by the PID_velocity.limit.

PID_velocity.limit is initialised to be equal to either current_limit or the voltage_limit in BLDCMotor.init(). So as long as you set the motor.voltage_limit before calling motor.init(), the effect is actually the same.

I agree that your suggestion is somehow "cleaner", and probably more correct. But it also means more MCU cycles will be used in each iteration, so I am not really sure we should change it.

I think actually we should leave it as it is for now, and re-think the entire initialisation process for a V3 or V4 release in the future. At the moment the initialisation is quite sensitive to the order you do things, and it's easy to get things wrong resulting in very confusing bugs for the user. So I think we should attack this topic as part of a more general improvement and leave it for now.

[runger1101001]

:-) looks like Antun and my comments crossed!

[greymfm]

Thanks for explanations - Then actually my problem was that we switch between velocity and torque control at runtime (each time with different phase_resistance paramters but without calling foc.init) and that did not update the PID_velocity.limits... I have done the PID_velocity.limits update in my own code now and that works. Let's regard this as closed.

[askuric]

Yeah, you guys are right. We would need to make a simpler method to change limits both in voltage_limit and in PID_velocity object. Maybe pass them as a reference to the PID class or something similar. However constraining the voltage outside the PID is not a good idea. As the PID antiwindup would become a big problem.

@runger1101001 you're right. There are some messy parts of the init that would require some refactoring :D And this is one of them.

BTW, here is a community thread having a similar theme: https://community.simplefoc.com/t/setting-current-limits-in-main-loop/1972/3