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

Adaptation to the Arduino IDE for ESP32 #3

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

Conversation

joseluu
Copy link

@joseluu joseluu commented Dec 27, 2018

Hi David,

I did adapt to the Arduino IDE which may be more easy/appealing to some (at least for me).

One change I would like to point out which I feel is an improvement is the line 192 of frequency_count.c:
frequency_hz = (count >> 1) / task_inputs->sampling_window_seconds;
where the division by 2 happens in integer mode, having the effect of rounding down the division instead of injecting a 0.5 value as when the division happens in double mode.
The problem is caused by counting the second edge of the signal and using it for the subsequent computation, rounding down eliminates the second edge. You can feel the difference if you measure a 10000Hz frequency in 0.001 window duration, in this cas, the display alternates between 10500 and 10000, if you measure in 0.01 window duration, the display alternates between 10050 and 10000. This change fixes this problem.

Best
Jose

@DavidAntliff
Copy link
Owner

@joseluu thank you for the PR, especially the second edge fix.

I will consider merging but can I ask you to put an appropriate license on your .ino file please? MIT or something compatible. Put your own name in there too if you want, because it's your work.

Also, does the Arduino component format mean we have to remove the doc and include directories? The include directory is useful to build as an ESP-IDF component, and the doc/Doxygen file is used to generate the API documentation. I don't want to remove these. Can the .ino simply live within the existing file structure?

@woodlist
Copy link

woodlist commented Jun 5, 2024

Mr. Jose, please, try to re-compile with newer ESP32 3.0.1 core in Arduino IDE 2.3.2

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