end2endzone / AnyRtttl

A feature rich arduino library for playing rtttl melodies
MIT License
5 stars 1 forks source link

Error compiling for board ESP32 Wrover Kit (all versions). #6

Closed ghmpi closed 1 month ago

ghmpi commented 1 year ago

Hello,

I have a code base I use mainly on two boards, a XIAO_ESP32C3 and a ESP32_WROVER_KIT. After adding the AnyRtttl library and compiling for the WROVER board, it fails. The ESP32C3 board compiles and works just fine! Any ideas? Error below...

I'm liking this library otherwise!

Thank you!

Regards, -Moses

C:\Users\Mo\Documents\Arduino\libraries\arduino_65006\src\anyrtttl.cpp:116:23: error: invalid conversion from 'void ()(uint32_t)' {aka 'void ()(unsigned int)'} to 'anyrtttl::DelayFuncPtr' {aka 'void (*)(long unsigned int)'} [-fpermissive] DelayFuncPtr _delay = &delay; ^~ Error compiling for board ESP32 Wrover Kit (all versions).

ghmpi commented 1 year ago

I believe I found the issue..

In anyrtttl.h...

//typedef void (DelayFuncPtr)(unsigned long); // ORIGINAL on v2.2.1 NOT WORKING ON WROOVER typedef void (DelayFuncPtr)(uint32_t); // NOW WORKING

end2endzone commented 1 year ago

Hi. I do not own an ESP32 or ESP8266. I am also unfamiliar with the Wroover kit. I do not have any mean to validate a change in code. If you feel you can help, I am open to push request.

Ironically, changing uint32_t by unsigned long is exactly the change I implemented in version 2.2.1. As a temporary fix, you may try version 2.2.0 instead and see if the problem is resolved. If it is, please let me know.

As a starting point, I think my other rtttl library, namely NonBlockingRtttl, is compatible with ESP32. For example, tone() and noTone() alternative functions were added here and here wrapped in a #if defined(ESP32).

As I said, I am open to push request and I would appreciate a contribution to get EPS32 compatibility.

ghmpi commented 1 year ago

@end2endzone

The development board I use is an original board from Espressif commonly known as the "ESP32-DevKitC" https://www.espressif.com/en/products/devkits/esp32-devkitc/overview The particular board I have uses the ESP32-WROVER chip This board is fairly common and a lot of clones are available as well. Generally any "ESP32" code works on it.

I did try version 2.2.0, but that gave more errors which I do not remember and did not troubleshoot as the error on 2.2.1 was simpler for me to understand :)

I will try the NonBlockingRTTTL. I did find that before, but I figured the AnyRtttl library was the new version.

From my research I believe the arduino-esp32 library is now using ledcWriteNote()... for the tone() function. Or maps the tone() function on esp32 board to that function. I'm not sure, as I'm just started to use these functions for audio output myself.

I will gladly help test any new version, or modify the code to work on ESP boards. I think maybe a #if defined(ESP32) or two should fix it.

On a separate note, the music it generates has small delays that are not in the RTTTL notes. I think it may have something to do with the PWM channel, I may have something interfering. I was going to dig into the code to see if I can change the channel and see if it fixes it. Your thoughts on this are apprecaited.

Your library has has helped me add an alarm to a clock kit I produce. Thank you for your work.

Regards, -Moses

end2endzone commented 1 year ago

@ghmpi

I apologize for the late reply.

I would like to add ESP32 support to AnyRtttl for the next version. I think I have a working library. I verified/compiled the library against the ESP32 core and no error or warning are showing in the logs. I do not have a physical ESP32 board to confirm. I created a sample sketch that can be used for validation. The sketch is available at https://github.com/end2endzone/AnyRtttl/blob/master/examples/ESP32Rtttl/ESP32Rtttl.ino. I would appreciate if you could look at it on real hardware.

To test ESP32 support, you may need to download the latest version of the library in .zip format. I did not yet published the latest changes to the Arduino Library Manager because I would like to include ESP32 support first.

From my research I believe the arduino-esp32 library is now using ledcWriteNote()... for the tone() function. Or maps the tone() function on esp32 board to that function. I'm not sure, as I'm just started to use these functions for audio output myself.

Yes. This is also how the sample sketch is written. It provides "custom" tone() and noTone() functions to AnyRtttl (see lines 15 to 29 for examples). These functions delegate to ledc family of functions. More specifically ledcWrite(), ledcWriteTone(), ledcSetup() and ledcAttachPin(). See ESP32Rtttl.ino, lines 15 to 29 for examples.

On a separate note, the music it generates has small delays that are not in the RTTTL notes. I think it may have something to do with the PWM channel, I may have something interfering. I was going to dig into the code to see if I can change the channel and see if it fixes it. Your thoughts on this are apprecaited.

I do not understand what you mean by "small delays". Could you be more specific? Do you mean there are delays "longer than expected" between the notes? The library is designed to be "silent" between each note (call noTone() function). See anyrtttl.cpp, line 262 for an example. This is required by design because the human ear need a clear cut in order to hear "the beginning of a new note" . If you don't, your ear will find something is wrong if you directly jump from 200hz to 300hz.

Could you record the Tetris melody sample on your phone and paste it in this issue? That would make it easier for me to understand what you mean.

Again, your help is much appreciated.

EDIT: typos.

EDIT: The ESP32 seems to have hardware PWM capabilities. For this reason, I would expect the library to behave as intended.

end2endzone commented 1 year ago

Unexpected delays could also be heard if your melody have unexpected characters. This could be reproduced if you try to play a melody spread on multiple lines (with carriage return characters) in the string.

For example, if you play the following

Halloween:d=4,o=5,b=180:8d6,8g,8g,8d6,8g,8g,8d6,8g,8d#6,8g,
8d6,8g,8g,8d6,8g,8g,8d6,8g,8d#6,8g,8c#6,8f#,8f#,8c#6,8f#,8f#,
8c#6,8f#,8d6,8f#,8c#6,8f#,8f#,8c#6,8f#,8f#,8c#6,8f#,8d6,8f#

you will end up with delay() calls that are not expected.

You can try to reproduce it yourself with the Rtttl2Code example.

geiseri commented 7 months ago

I believe I found the issue..

In anyrtttl.h...

//typedef void (DelayFuncPtr)(unsigned long); // ORIGINAL on v2.2.1 NOT WORKING ON WROOVER typedef void (DelayFuncPtr)(uint32_t); // NOW WORKING

It looks like the arduino-esp32 implementation uses uint32_t. A short look around makes it look like it is in the minority🤣. I guess you need a HAL for their HAL.

dtdobney commented 3 months ago

Hi end2endzone Great library thanks.

Thanks ghmpi

I can confirm the fix works with my (physical) [ESP32-WROOM-32U ESP32-DevKitC]

I was getting this error, similar to ghmpi: *error: invalid conversion from 'void ()(uint32_t)' {aka 'void ()(unsigned int)'} to 'anyrtttl::DelayFuncPtr' {aka 'void ()(long unsigned int)'} [-fpermissive] DelayFuncPtr _delay = &delay;**

When I make the suggested change I can get the example code to work (tetris, mario, etc.) typedef void (DelayFuncPtr)(unsigned long); // ORIGINAL on v2.2.1 NOT WORKING ON WROOVER typedef void (DelayFuncPtr)(uint32_t); // NOW WORKING

thanks for the support!

end2endzone commented 1 month ago

@ghmpi, @geiseri and @dtdobney. I have created release 2.3.0 which includes fixes to resolve this issue. It should be available in Arduino Library Manager in a few days. Hope you like it.

Like I said, I do not own an ESP32, can someone confirm the library is compatible with an ESP32 and working as expected? Thank you.

Also, if you do not want to wait until it is available to download from the Arduino IDE, you can also download a copy directly from github at https://github.com/end2endzone/AnyRtttl/archive/refs/tags/2.3.0.zip.