Skip to content
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

Fix "missing interrupt" and truncated register reads in TDC7200 driver code #31

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

thinkfat
Copy link

@thinkfat thinkfat commented Sep 8, 2020

This pull request fixes two bugs in the tdc7200 driver, the first one concerning the missing interrupts for measurement completion, the second one about truncated 24 bit register reads. Please consider merging them with your master branch.

Matthias Welwarsky added 2 commits September 8, 2020 17:15
The TDC7200 code uses a bogus work-around for not receiving
an interrupt for a completed measurement. Fixed by actually
enabling the necessary interrupt and adding code to acknowledge
and clear all interrupt conditions after reading each measurement

Signed-off-by: Matthias Welwarsky <[email protected]>
In the readReg24() method, the variable "msb" is a 16bit unsigned int
which gets shifted beyond its size, effectively clearing the MSB of
each register read. Fix by casting it to an uint32_t.

Signed-off-by: Matthias Welwarsky <[email protected]>
@arodland
Copy link

arodland commented Oct 6, 2020

These both look good to me.

Copy link
Member

@n8ur n8ur left a comment

Choose a reason for hiding this comment

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

Hi Matthias --

Finally doing a bit of maintenance and I have made the correction to readReg24() you proposed but I did it via manual patch rather than pull. Per our email messages, I'm not making the interrupt changes, but still plan to investigate whether there's something stupid that I've been missing that causes the problem.

Thanks!
John

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.

3 participants