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

tone generation with ledcSetup and ledcWrite instable under fw 2.0 (was fine with 1.0.6) #6199

Closed
1 task done
bongoo1 opened this issue Jan 27, 2022 · 12 comments
Closed
1 task done
Assignees
Labels
Area: Peripherals API Relates to peripheral's APIs. Resolution: Expired More info wasn't provided
Milestone

Comments

@bongoo1
Copy link

bongoo1 commented Jan 27, 2022

Board

esp32dev

Device Description

piezzo beeper connected to gpio33

Hardware Configuration

nothing relevant

Version

v2.0.0

IDE Name

platformIO

Operating System

win10

Flash frequency

default

PSRAM enabled

no

Upload speed

115200

Description

i generate short tones when pressing a key on the keyboard connected using the commands ledcSetup and ledcWrite to do pwm on the pin, connected to the piezzo beeper.
this works fine under arduino framework 1.0.6.
after switching to framework 2, tone generation got instable.
only about every second time, i hear a beep, although the code that should beep is executed every time.
when going back to 1.0.6, this works without any issues.

Sketch

ledcSetup(1,BEEP[beepRptr].freq,8);
        ledcWrite(1,128);

Debug Message

no error message, just no output signal (beep)

Other Steps to Reproduce

No response

I have checked existing issues, online documentation and the Troubleshooting Guide

  • I confirm I have checked existing issues, online documentation and Troubleshooting guide.
@bongoo1 bongoo1 added the Status: Awaiting triage Issue is waiting for triage label Jan 27, 2022
@P-R-O-C-H-Y
Copy link
Member

Hi @bongoo1,
Can you provide complete Sketch to reproduce the issue? Or at least the part with the keyboard.

@bongoo1
Copy link
Author

bongoo1 commented Jan 28, 2022

here's the sketch as it actually looks like after i tried a few things more.

with platformio.ini containing
platform = espressif32
it builds with framework 1.0.6
then it works fine.

with platformio.ini containing

platform = https://github.com/platformio/platform-espressif32.git#feature/arduino-upstream
platform_packages =	platformio/framework-arduinoespressif32 @ https://github.com/espressif/arduino-esp32.git

instead, there is not beep audible about half of the time.

with the code below, call
beepShort750()
to add a beep to the beep sequence queue.
the
beepDo()
is then periodically called from within the loop to play the beep sequence from the queue.
this allows me to also configure a melody of a few seconds without blocking the execution of other code.


typedef struct beepType {
  int freq;    //0: off, 1: full, all others: frequency
  int length;  //length in ms
} beepType;

#define beepPin 33

#define beepSize 128
beepType BEEP[beepSize] = {0};
int beepWptr = 0;
int beepRptr = 0;

void setupBuzzer() {

    ledcSetup(1,1000,8); //channel 1, 1khz, 0..255
    ledcAttachPin(beepPin,1); //prepare pwm output, gpio33 from channel 1
    ledcWrite(1,0); 
}

void beepAdd(int freq, int length) {
//add beep to queue

  if ((beepWptr + 1) == beepRptr) {
    //beep queue full
  } else {
    BEEP[beepWptr].freq = freq;
    BEEP[beepWptr].length = length;
    beepWptr++;
    if (beepWptr == beepSize) {
      beepWptr = 0;
    }
  }
}

void beepDo() {
// beep if required
  
  static long unsigned int endBeep = 0;
  if (millis() > endBeep) { //beep phase ended
    if (beepRptr != beepWptr) { //beep command in queue
      if (BEEP[beepRptr].freq == 0) {
        ledcWrite(1,0); //turn off
        Serial.printf("B:off, ");
      } else if (BEEP[beepRptr].freq == 1) {
        ledcWriteTone(1,100);
        ledcWrite(1,254); //turn fully on (nominal freq of beeper)
        Serial.printf("B:max, ");
      } else {
        ledcWrite(1,128);
        ledcWriteTone(1,BEEP[beepRptr].freq);
        Serial.printf("B:%d, ",BEEP[beepRptr].freq);
      }
      endBeep = millis() + BEEP[beepRptr].length;
      Serial.printf("(%u/%u)\n",millis(),endBeep);
      beepRptr++;
      if (beepRptr == beepSize) {
        beepRptr = 0;
      }
    } else { //no new beep data, stop beeper
      ledcWrite(1,0); //turn off
    }
  }
}


void beepShort750() {

  beepAdd(750,100);
  beepAdd(0,0);
}

@P-R-O-C-H-Y P-R-O-C-H-Y self-assigned this Jan 28, 2022
@VojtechBartoska VojtechBartoska added Area: Peripherals API Relates to peripheral's APIs. and removed Status: Awaiting triage Issue is waiting for triage labels Feb 1, 2022
@VojtechBartoska VojtechBartoska moved this from Todo to Under investigation in Arduino ESP32 Core Project Roadmap Feb 2, 2022
@cjcr
Copy link

cjcr commented Feb 3, 2022

I can confirm this issue. Is ok with 1.0.6.

@PilnyTomas PilnyTomas self-assigned this Mar 7, 2022
@VojtechBartoska VojtechBartoska moved this from Under investigation to In Progress in Arduino ESP32 Core Project Roadmap Mar 9, 2022
@P-R-O-C-H-Y
Copy link
Member

P-R-O-C-H-Y commented Mar 22, 2022

Hi @bongoo1 , @cjcr

I have tested your sketch with Arduino-esp32 master branch and everything is working great :)
Can you test your code with master branch or version 2.0.2? Ledc was refactored to use ESP-IDFs api.

Some points to your sketch:
I did some improvements for your beepDo() function.

void beepDo() {
// beep if required
  
  static long unsigned int endBeep = 0;
  if (millis() > endBeep) { //beep phase ended
    if (beepRptr != beepWptr) { //beep command in queue
      if (BEEP[beepRptr].freq == 0) {
        ledcWrite(1,0); //turn off
        Serial.printf("B:off, ");
      } else if (BEEP[beepRptr].freq == 1) {
        //ledcWriteTone(1,100); better to use change freq function that will not change the duty as ledcWriteTone() do.
        ledcChangeFrequency(1,100,8); //<-- use this instead
        ledcWrite(1,254); //turn fully on (nominal freq of beeper)
        Serial.printf("B:max, ");
      } else {
        //ledcWrite(1,128); <-- no reason to call this, the ledCWriteTone() sets the duty automatically to 128
        ledcWriteTone(1,BEEP[beepRptr].freq);
        Serial.printf("B:%d, ",BEEP[beepRptr].freq);
      }
      endBeep = millis() + BEEP[beepRptr].length;
      Serial.printf("(%u/%u)\n",millis(),endBeep);
      beepRptr++;
      if (beepRptr == beepSize) {
        beepRptr = 0;
      }
    } else { //no new beep data, stop beeper
      ledcWrite(1,0); //turn off
    }
  }
}

If you still have some issue let me know :)

@P-R-O-C-H-Y P-R-O-C-H-Y added the Resolution: Awaiting response Waiting for response of author label Mar 22, 2022
@VojtechBartoska VojtechBartoska added this to the 2.0.3 milestone Mar 23, 2022
@bongoo1
Copy link
Author

bongoo1 commented Mar 29, 2022

hi @P-R-O-C-H-Y
thanx for testing!
my actual settings in the platformio.ini are:

[env:esp32dev]
platform = https://github.com/platformio/platform-espressif32.git#feature/arduino-upstream
board = esp32dev
framework = arduino
platform_packages =
	platformio/framework-arduinoespressif32 @ https://github.com/espressif/arduino-esp32.git
monitor_speed = 115200
build_flags = -DCORE_DEBUG_LEVEL=2
lib_deps = 
	bodmer/TFT_eSPI@^2.4.25
	sstaub/NTP@^1.4
	marian-craciunescu/ESP32Ping@^1.7

does this take version 2.0.2 or what do i need to change there?
unfortunately i do not really understand this version settings, as whatever i tried to select there did not work.

and about your modification to the code:
what i wanna do with
ledcWriteTone(1,100);
is to fully set the output to high (without frequency) to run the beeper at it's native frequency when it's just connected to 3v3.
so does your modification really do this?

thanx for your help!

@P-R-O-C-H-Y
Copy link
Member

Actually I am not familiar with PlatformIO settings. I am using Arduino IDE / VS code. But what you can do is to check our docs or this issue link. But I think you are using just 2.0 version of Arduino-esp32. Follow the docs or issue to get latest version ;)

If you need some info, what the functions do you can check our docs, ledc api was added recently: ledc docs

But some quick explanation for what I understand in your code, I will separate the BeepDo() to some parts:

  • Here you want to set output to LOW:
     if (BEEP[beepRptr].freq == 0) {
           ledcWrite(1,0); //turn off
           Serial.printf("B:off, ");
      } 
  • Here you want to set output to HIGH with freq 100Hz (do you need to change freq? when output is always high?):
      else if (BEEP[beepRptr].freq == 1) {
           //ledcWriteTone(1,100); //<-- THIS will set duty to 50%, but you want always high so no need to call this.
           ledcChangeFrequency(1,100,8); //<-- better to use this instead to change only frequency
           ledcWrite(1,254); //<-- Are you set to HIGH (maybe use 255 instead of 254) //turn fully on (nominal freq of beeper)
           Serial.printf("B:max, ");
      } 
  • Here you want to set duty to 50% and selected freq. The ledcWrite() is not needed because ledcWriteTone does the 50% duty automatically as I commented before.
      else {
           //ledcWrite(1,128); <-- no reason to call this, the ledCWriteTone() sets the duty automatically to 128
           ledcWriteTone(1,BEEP[beepRptr].freq);
           Serial.printf("B:%d, ",BEEP[beepRptr].freq);
      }

Hope it helps, but feel free to ask for more help :)

@bongoo1
Copy link
Author

bongoo1 commented Mar 29, 2022

hi @P-R-O-C-H-Y

i too thought that i probably use 2.0.0, but could also be 2.0.2. i was not able to find that out. that's why i asked ;-)

and about the code:
yes, first part is turning off by keeping the output low.
second is turning on at nominal frequency by keeing the output high. i tried to do so by modifying the duty cycle accordingly. should i do here just a
ledcWrite(1,255);
? i was not sure if 255 would work for pwm or if the maximum is 254.
and the 3rd one writes out a frequeny with 50% duty.

@P-R-O-C-H-Y
Copy link
Member

Hi @bongoo1

i too thought that i probably use 2.0.0, but could also be 2.0.2. i was not able to find that out. that's why i asked ;-)

I think according to your platformio.ini you use 2.0.0. But if change this it should do the thing ;)

platform_packages =
	platformio/framework-arduinoespressif32 @ https://github.com/espressif/arduino-esp32.git 

to

platform_packages =
	platformio/framework-arduinoespressif32 @ https://github.com/espressif/arduino-esp32.git#master

second is turning on at nominal frequency by keeing the output high. i tried to do so by modifying the duty cycle accordingly. should i do here just a ledcWrite(1,255); ? i was not sure if 255 would work for pwm or if the maximum is 254.

Actually in 2.0 version I don't know if 254 or 255 will do it for you. In 2.0.2 the LEDC was refactored and now the 255 will do the thing for you.
But you want to set output to HIGH and change the frequency to best option is to call ledcChangeFrequency(1,100,8); and after that ledcWrite(1,254); (or 255).

and the 3rd one writes out a frequeny with 50% duty.
Thats what ledcWriteTone(1,BEEP[beepRptr].freq); do. No need to change the duty manually by ledcWrite(1,128);

Your code will work but it can be simplified and if the execution is slower or some interrupt happens there might be some hazards when you want to set fully ON (HIGH) but you call ledcWriteTone(1,100); first that's sets duty to 50%.

@bongoo1
Copy link
Author

bongoo1 commented Mar 29, 2022

hi @P-R-O-C-H-Y
after switching platform_packages, i get the error message

"C:\\Users\\tinu\\.platformio\\packages\\framework-arduinoespressif32\\tools\\sdk\\esp32\\include\\config\" wurde nicht gefunden.",

i get this entry for each project where i switch the platform_packages.
is this just something to ignore or is something missing in my setup?
thanx!

@P-R-O-C-H-Y
Copy link
Member

Hi @bongoo1 ,
I am sorry but I am not familiar with the PlatformIO. Actually I never used that.
I thought that only adding the master will solve that. But please do thru the docsI linked in comments before. It has to work ;)

@VojtechBartoska
Copy link
Contributor

any updates @bongoo1?

@VojtechBartoska VojtechBartoska moved this from In Progress to In Review in Arduino ESP32 Core Project Roadmap Apr 20, 2022
@P-R-O-C-H-Y
Copy link
Member

Hello,

as there was no answer in 14 days, I'm closing the issue as expired to keep our backlog manageable.

If it's still needed, please reopen the issue.

Thanks for understanding.

Repository owner moved this from In Review to Done in Arduino ESP32 Core Project Roadmap Apr 25, 2022
@P-R-O-C-H-Y P-R-O-C-H-Y removed the Resolution: Awaiting response Waiting for response of author label Apr 25, 2022
@P-R-O-C-H-Y P-R-O-C-H-Y added the Resolution: Expired More info wasn't provided label Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Peripherals API Relates to peripheral's APIs. Resolution: Expired More info wasn't provided
Projects
Development

No branches or pull requests

5 participants