simplefoc / Arduino-FOC

Arduino FOC for BLDC and Stepper motors - Arduino Based Field Oriented Control Algorithm Library
https://docs.simplefoc.com
MIT License
1.95k stars 511 forks source link

[FEATURE] Two issues with MagneticSensorPWM #348

Closed sylque closed 2 months ago

sylque commented 7 months ago

Issue 1

Unless I'm missing something, the getRawCount() method should be made public, otherwise it is not possible, in client code, to measure the _min and _max values required by MagneticSensorPWM(_pinPWM, _min, _max).

Issue 2

Line 88 of MagneticSensorPWM.cpp reads as follow:

pulse_length_us = pulseIn(pinPWM, HIGH, 1200); // 1200us timeout, should this be configurable?

Yes, I confirm the 1200us timeout should be configurable: in my case, I needed 1400us to get my AS4850A to work.

runger1101001 commented 7 months ago

Thanks for reporting it.

If you're using STM32 MCUs, you might want to look at this implementation also: https://github.com/simplefoc/Arduino-FOC-drivers/tree/master/src/utilities/stm32pwm

We'll try to make these changes for an upcoming release, but really we encourage users to use other sensor types than PWM... the slow update time will limit your performance.

sylque commented 7 months ago

Thanks @runger1101001 . I will switch to SPI as soon as possible, but the possibility to first test the hardware with PWM is very useful. Indeed, some magnetic encoders come with only the PWM output ready to plug. For example, in my current case (miniature combo BLDC+AS5048A), wiring the SPI output will require: 1. to cut the aluminium casing to make room for wires, and 2. to solder wires on very small copper tracks.

runger1101001 commented 7 months ago

So for Issue #1 there actually already is a way:

Call sensor.update() to read new sensor values, and then you can read the sensor.pulse_length_us to get the values you need.

For Issue #2 I have just committed changes to make this a configurable setting, sensor.timeout_us.