search

bitfasching / AS5601

Library for driving the sensor AS5601 over I²C on an Arduino
Other
16 stars 6 forks source link

Compile fails with fresh Arduino & Espressif board library #2

Closed ed-french closed 3 years ago

ed-french commented 5 years ago

Fresh arduino install. Install Espressif ESP32 board library. Set board to ESP32 Pico Kit. Compile fails with error that it can't find a matching function for max (unsigned &int,int) on line 180.

Adding explicit cast solves compile problem: angleSteps = min( max( int(angleSteps), 8 ), 2048 );

bitfasching commented 5 years ago

Hi @ed-french, thanks for pointing out. max() should be defined in the Arduino headers. Perhaps this is actually a bug in the ESP library? The only thing that keeps confusing me is that stray ampersand in the error message. (If you supplied a reference to a number instead of the number itself, the error would actually be reasonable.) Did you verify if your fix just removes the compiler error or whether it makes setResolution() working?

Anyway, if you want to fix this in the AS5601 library, it's not necessary to cast the input if we can cast the constants with a U suffix:

// coerce angle steps to supported values (8, 16, 32, …, 2048)
angleSteps = min( max( angleSteps, 8U ), 2048U );
bitfasching commented 3 years ago

Hi @ed-french, any update so far?

ed-french commented 3 years ago

Hi, sorry for the lack of update. Shortly after this we found problems with the approach we were using and we ended up making our own sensor instead(!). If anyone else has this problem I could dig out the old code and see if it compiles- there'll have been a ton of updates to the ESP32 library since then- which sounded like the culprit anyway.

bitfasching commented 3 years ago

Wow, alright! I agree, the library changed anyway, so we don't need to follow up on the old case. If the issue persists with the current state of the library, someone will hopefully open a new issue. Thanks for the quick reply.