Skip to content

Conversation

@xp-dv
Copy link
Contributor

@xp-dv xp-dv commented Sep 27, 2024

Example 3 Patch:

  • Removed unneeded I2C address check
  • Fixed distance calculation
  • Fixed incorrect delay causing sensor to read 0 and delays verified using Siglent SDS2104X Plus oscilloscope
  • Added clarifying comments
  • Tested for function using recently purchased SEN-24805 on the Arduino Nano

…on, fixed incorrect delay causing sensor to not read, added clarifying comments, tested for function
@xp-dv
Copy link
Contributor Author

xp-dv commented Sep 28, 2024

I can also add the oscilloscope capture png to the project folder in a second commit, if that would be helpful for users.

@xp-dv
Copy link
Contributor Author

xp-dv commented Sep 29, 2025

@gigapod Any reason why this hasn't been merged yet?

@gigapod
Copy link
Member

gigapod commented Sep 29, 2025

HI @xp-dv -

Thanks for the PR!

While I did update this library by migrating to the SparkFun Toolkit, updating the repo layout and adding Doxygen support, I don't monitor this repo - so I didn't see this PR.

That said, I'll take a look and review the PR. Looks like the original implementation could use some tweaks and cleanup.

NOTE: Once the code looks good, we'll still need to validate operation before merging. This can take up to a month right now.

-Kirk

Copy link
Member

@gigapod gigapod left a comment

Choose a reason for hiding this comment

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

operationally makes sense. The original engineer on this made a mistake by keeping I2C logic/functionality in this example.

We still need to test this before merging and publishing the changes. ..

-K

const float speedOfSound = 340.00; // Speed of sound in m/s
const float millisPerSecond= 1000.00; // Number of milliseconds in a second

void setup() {
Copy link
Member

@gigapod gigapod Sep 29, 2025

Choose a reason for hiding this comment

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

  • Agree we can simplify the constant to eliminate the calculation in the loop. But the math behind this should be in the comments. I'd also rename the const to include units

@gigapod gigapod merged commit e9b8e35 into sparkfun:main Oct 13, 2025
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