ArduinoSound icon indicating copy to clipboard operation
ArduinoSound copied to clipboard
verified publisher icon arduino-libraries

Change amplitude example to output values in dB

Open sandeepmistry opened this issue 6 years ago • 6 comments

Suggestion from @tigoe.

sandeepmistry avatar Apr 17 '19 13:04 sandeepmistry

@sandeepmistry could you please elaborate on this? What unit does the output show currently?

ashutoshshrm529 avatar Mar 17 '20 19:03 ashutoshshrm529

I would like to fix this issue. Is anyone on it already?

MisterAwesome23 avatar Mar 21 '20 21:03 MisterAwesome23

@MisterAwesome23 there is an existing pull request to fix this: https://github.com/arduino-libraries/ArduinoSound/pull/24 Feel free to test it out and review it.

per1234 avatar Mar 21 '20 23:03 per1234

I feel it can be improved minutely.

  • No need to use abs(), two reasons, technically sound can be negative decibels also, the amplitudeAnalyzer.read() should ideally return a non negative value so no need of abs()

Should I create my own pull request or suggest the changes there?

MisterAwesome23 avatar Mar 22 '20 22:03 MisterAwesome23

@MisterAwesome23 I think it would be best to start by suggesting your changes on the existing pull request. The reason is that it is more convenient for the repository maintainers to have only a single pull request to review and merge, rather than having to evaluate two different PRs to decide which should be merged and which closed.

If there is a situation such as you and the PR author having differences of opinion, then it would be great if you would submit a PR.

Thanks!

per1234 avatar Mar 22 '20 22:03 per1234

Sure. Makes sense to keep a single pull request. I'll suggest my changes there. Thanks :)

MisterAwesome23 avatar Mar 22 '20 23:03 MisterAwesome23