UncleRus / esp-idf-lib

Component library for ESP32-xx and ESP8266
https://esp-idf-lib.readthedocs.io/en/latest/
1.39k stars 431 forks source link

feat: added anemometer(wind speed) sensor #611

Closed QB4-dev closed 7 months ago

QB4-dev commented 7 months ago

This is driver for wind sensors like this one: https://www.amazon.com/TOPINCN-Speeds-Anemometer-Environment-Anemometers/dp/B083JFLNR7

Tested on ESP8266

UncleRus commented 7 months ago

In general, I don’t really like this component that works on counting GPIO interrupts. This approach is still quite appropriate for the ESP8266, but the ESP32 has a hardware pulse counter.

If the component supported PCNT for esp32 and used GPIO interrupts as a fallback for esp8266, this would be a different matter.

In its current form, this component will not be included in the library, sorry.

QB4-dev commented 7 months ago

In general, I don’t really like this component that works on counting GPIO interrupts. This approach is still quite appropriate for the ESP8266, but the ESP32 has a hardware pulse counter.

If the component supported PCNT for esp32 and used GPIO interrupts as a fallback for esp8266, this would be a different matter.

In its current form, this component will not be included in the library, sorry.

You are absolutely right. When I will have access to ESP32, I'll add ESP32 hardware pulse counter. Please leave this PR as is until I add ESP32 part

QB4-dev commented 7 months ago

I have modified this driver according to your suggestions but not going very well...

There are two significant problems:

  1. ESP32C3 has no hardware PCNT so I cannot use PCNT, but it is still ESP32 and HELPER_TARGET_IS_ESP32 is not enough to setup interrupt based PPS counter. I disabled ESP32C3 support for now.

Maybe a macro like this will be good enough?

#define ESP_PCNT_SUPPORTED HELPER_TARGET_IS_ESP32 && !defined(CONFIG_IDF_TARGET_ESP32C3)
  1. ESP-IDF version < 5.0 uses old PCNT driver so it may be easier to fallback to ESP8266 style approach than writing new code with outdated driver
QB4-dev commented 7 months ago

I decided to add described above changes.

Now driver is using PCNT on all ESP32 chips except ESP32C3 and on ESP-IDF v5.0.0 and above, and interrupts on ESP8266 and with older ESP-IDF

UncleRus commented 7 months ago

That's a horse of another color!

But please run clang-format on your code. Then add a documentation page for your component to /docs/source/groups. Look at existing documentation pages to understand how to do this. Don't forget to add a link to your documentation page in the table of contents (/docs/source/index.rst).

QB4-dev commented 7 months ago

Ok. I am gonna add documentation and of course run clang-format on my source files(sorry about that). It may be little late but how about to rename this driver to 'impulse-sensor'? I have written it for anemometr, but it may be useful for variety of sensor with impulse output. For example fluid flow meters, shaft rotation sensors etc.

UncleRus commented 7 months ago

No problem, you can rename it

QB4-dev commented 7 months ago

OK. Ready to review

UncleRus commented 7 months ago

Thank you!