Skip to content

[FEATURE] Two issues with MagneticSensorPWM #348

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

Closed
sylque opened this issue Nov 28, 2023 · 3 comments
Closed

[FEATURE] Two issues with MagneticSensorPWM #348

sylque opened this issue Nov 28, 2023 · 3 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@sylque
Copy link

sylque commented Nov 28, 2023

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.

@sylque sylque added the enhancement New feature or request label Nov 28, 2023
@runger1101001
Copy link
Member

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
Copy link
Author

sylque commented Nov 29, 2023

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 pushed a commit to runger1101001/Arduino-FOC that referenced this issue Dec 2, 2023
@runger1101001
Copy link
Member

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.

@runger1101001 runger1101001 added this to the 2.3.3_Release milestone Dec 2, 2023
runger1101001 added a commit that referenced this issue Dec 2, 2023
Make timeout configurable in MagneticSensorPWM #348
@runger1101001 runger1101001 self-assigned this Dec 2, 2023
@askuric askuric closed this as completed Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants