-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
fix for AJ-SR04M sensor #4637
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
base: main
Are you sure you want to change the base?
fix for AJ-SR04M sensor #4637
Conversation
When using AJ-SR04M sensor, pulseIn function always returns 0 when timeout value is about 5-10 thousands. I don't know why.
WalkthroughThe pull request modifies the logic of the Changes
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Please thoroughly check on all supported sensors.
@@ -152,8 +152,8 @@ class Animated_Staircase : public Usermod { | |||
delayMicroseconds(2); | |||
digitalWrite(signalPin, HIGH); | |||
delayMicroseconds(10); | |||
digitalWrite(signalPin, LOW); |
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.
This line is required to drop signal pin LOW and end transmission. Please revert.
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.
you're right.
It was mistakenly deleted
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.
I've reverted this line and committed this fix as well. Please, have a look.
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.
Currently I have no way of testing it but I think adding 30ms wait for response may be a bit long and can produce visible hiccups in LED display (default FPS is 42 or about 23ms between frames).
Even though approved I would be cautius with the merge and would require thorough testing before doing so (using both sensors).
I will test it in coming weeks. I will update you with the results within this thread. |
When using AJ-SR04M sensor, pulseIn function always returns 0 when timeout value is about 5-10 thousands. I don't know why.
With HC-SR04 behavior is different - it returns real value. So to support both sensors, I've added this additional logic:
unsigned long duration = pulseIn(echoPin, HIGH, 30000);
return (duration > 0 && duration < maxTimeUs);
Summary by CodeRabbit
Refactor
Style