Skip to content

Fix always zero AnalogIn on nordic#9114

Merged
tannewt merged 2 commits into
adafruit:9.0.xfrom
dhalbert:fix-nordic-analogin
Apr 1, 2024
Merged

Fix always zero AnalogIn on nordic#9114
tannewt merged 2 commits into
adafruit:9.0.xfrom
dhalbert:fix-nordic-analogin

Conversation

@dhalbert

@dhalbert dhalbert commented Mar 29, 2024

Copy link
Copy Markdown
Collaborator

This is quite mysterious. The underlying code hasn't changed in quite a while. nordic AnalogIn.value was always returning zero on 9.x.x builds, including some early betas. The problem starts appearing when gcc13 starts being used. Compiling with gcc10 or gcc12 causes the problem to go away.

While debugging, I added a printf which also caused the problem to go away. Adding tiny delay calls almost anywhere also helped. Eventually I realized it may have been due to these calls acting as an optimization memory fence. Adding asm volatile("": : : "memory"); had the same effect. I'm not sure whether this is a compiler bug or something else, but the fix works.

Also removed the logic to start with value = -1, which didn't seem necessary.

@dhalbert
dhalbert force-pushed the fix-nordic-analogin branch from d7df835 to 1821a7a Compare March 29, 2024 02:10
@dhalbert
dhalbert force-pushed the fix-nordic-analogin branch from 1821a7a to 453c82c Compare March 29, 2024 02:26

@tannewt tannewt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question

Comment thread ports/nrf/common-hal/analogio/AnalogIn.c Outdated
@dhalbert
dhalbert requested a review from tannewt March 30, 2024 21:07

@tannewt tannewt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants