khoih-prog/ESP32_New_TimerInterrupt

Suggestions - Main code is in header files / clearer documentation of the 3 timer library differences

tchilton opened this issue · 2 comments

Hi Khoi,
Nice library - thanks.

In your projects you keep all the main program code in header files rather than using the normal convention of using .h files for headers and .c files for c code (or .hpp for c++ headers and .cpp for c++ code.

Similarly, in your documentation there are some recommendations to only including header files once - they can automatically only include themselves at the right times with the standard #ifndef / #define constructs used in all the standard libraries and this is necessary and normal for more complex project with multiple modules to get the function prototypes visible in the right places.

It would be a lot less confusing if your code followed the normal C / C++ standards, rather than putting all the real code in the *-impl.h (implementation) header file.

I also note that you don't include your own headers in your implementation files. The norm would be to have the header file .h or .hpp included in the same named .c or .cpp file so that things like function prototypes are available, etc.

All the above could make the code a lot clearer, unless I missed a reason why the non-standard approach is used in your projects ?

Can I also suggest that in response to issue #7, that the README.md for each of the 3 timer libraries explains what is in the response to #7, since is a bit confusing at the moment. Ideally this should be in a prominent place near the top of the file, this is particularly important since only the ESPTimerInterrupt project shows on your front page meaning that many may not even be aware of the alternatives. The same logic applies for the text that appears in the Arduino libraries manager as it doesn't show the framework version / decision tree on which one to use there either.

Thanks.

Hi @tchilton

Thanks for your interest and time spent to research and investigate.

But I'm sorry I have to stick with the h-only implementation because of too many benefits. I leave you to research around, especially there are many discussions in my other libraries about the reasons.

Can I also suggest that in response to issue #7, that the README.md for each of the 3 timer libraries explains what is in the response to #7, since is a bit confusing at the moment.

Sorry again that I wish I had enough time and interests to update those README.md.

Certainly your contribution to add those info will be very welcome. Please help.

Best Regards,

Hi Khoi,

You have nearly 1000 repos on github, so if you want me take a look at your "too many benefits" that are buried in the discussion / issues in some of them, then you are going to have to narrow it down a bit. I'm not going to waste time looking through all of them just in case. If this is a common topic, then perhaps save us all time and open a discussion on it / add a pointer in the documentation.

As to the documentation, you have 3 ESP timer libraries and only gave info about two of them in your response to #7, so even if I was inclined to help you, there is insufficient information there to make that practical. Is the ESP S2 timer library now redundant ? Will the original one die now that the Arduino framework upgrades everyone to the v2.x Espressif codebase during board updates ? Does the v1.0 based timer framework work properly on the 2.x framework ?

Similarly, I can't control what goes on your front page on Github, that's one for you alone. Same applies for what is shown on the Arduino framework, again that is yours alone.

Its really hard to help you if you don't provide the relevant info - we can't see inside your mind :-)

Regards
Tim