-
Notifications
You must be signed in to change notification settings - Fork 12
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
RSDK-5903 Add micro-rdk support for HC-SR04 type ultrasonic sensors #136
Conversation
This is working well enough that I thought it was worth getting a review started. There are a few small TODOs left, and a few other things I'll leave comments about. For now, this doesn't use the mcpwm subsystem, it just manually times the pulse based on digital interrupts. I'd suggest that if we want to move to a mcpwm based approach that we land this implementation first in order to unblock using this sensor with the micro-rdk. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done with first pass
Meanwhile, I'm getting a clippy error I don't totally understand:
I really am using the
block into
So, I'm not sure where I should be doing this (or doing something else). |
@gvaradarajan pointed out to me on slack that I hadn't guarded the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wanted a minor change to the doc comment, but pending the executor conversation LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I filed two new tickets and added TODO's referencing them:
|
08e49bb
to
51112dc
Compare
This reverts commit 185c695.
No description provided.