Piezo doesn't behave well with setWatch()

Hi, I programmed my MDBT42Q module to make a small beep sound whenever I press or release a button. This is the code for my piezo sound effects:

function beep(f) { 
  if (f===0) digitalWrite(spk,0);
  else analogWrite(spk, 0.5, { freq: f } );
}

function buttonBoop(f) {
  beep(f);
  setTimeout(function() {digitalWrite(spk, 0);}, 50);
}

pretty simple, right?

So I tried to put them into setWatch functions so that my wristwatch(the PCB that houses the MDBT42Q module) can make a small beep whenever a button is pressed. The two setwatches are for when the button is pressed and released. When the button is pressed, the watch displays the hour on its two 7-seg LEDs for as long as the button is held down. when the button is released, the watch displays the minutes for 300 milliseconds. Here is the code:

setWatch(function(e) {
  data();  // grabs date & time
  buttonBoop(650); // supposed to beep piezo at 650hz for 50 milliseconds
  while (digitalRead(select) != 1) { // the watch displays the hour
    display(1, hourFirstDigit);
    display(2, hourSecondDigit);
  }
  allClear(); // resets 7-segment displays
}, select, {repeat: true, debounce: 50, edge: "falling"});

setWatch(function(e) {
  data(); // grabs date & time
  buttonBoop(950); // supposed to beep piezo at 950 hz for 50 milliseconds
  for (i = 0; i < 30; i++) { // watch displays the minutes
    display(1, minFirstDigit);
    display(2, minSecondDigit);
  }
    allClear(); // resets the 7 seg LEDs
}, select, {repeat: true, debounce: 50, edge: "rising"});
}

the problem is, whenever I press/release the button, the piezo speaker beeps until the LEDs are cleared. That is, when the button is pressed, the piezo beeps at 650hz until I release it, and when the button is released, the piezo beeps at 950mhz for 300mhz - which is when the LEDs are cleared. Sorry if this sounds confusing for you, I can't find a way to word it well.

I only want the piezo to beep for 50 milliseconds, what is making it go for the entire setWatch() duration?

The strange thing is, when do a clearWatch() and run this instead...

setWatch(function(e) {
  buttonBoop(650);
}, select, {repeat: true, debounce: 50, edge: "falling"});

setWatch(function(e) {
  buttonBoop(950);
}, select, {repeat: true, debounce: 50, edge: "rising"});

the piezo behaves just fine. A short beep when the button is pressed, and another short beep when the button is released. It's the same setWatch() code but with all the LED/time related code removed. But I can't find anything with my LED/time code. Thought this would help.

This is the entire source code for my watch. The buggy part is near the end.

Thanks.

I would not have both watches active at the same time... the bouncing messes... they cannot be active at the same time anyway. Make it a state machine:

1. single watch press
   
2. on press, check after some time if still pressed by reading, and if not go back initial state (1.)
   
3. start the beep and give it a timeout to end
   
4. single watch release
   
5. on release, start other beep with timeout and go back initial state after some time.
   
   

Since things may go faster - very short press - you may clear the first beep's timeout and also the beep.

Try 'both' and use use state to decide the frequence

You can find all the details in den api

https://www.espruino.com/Reference#l__global_setWatch

the problem is, whenever I press/release the button, the piezo speaker
beeps until the LEDs are cleared

That's what you program is supposed to do. There is no multitasking in Javascript, just a single thread. As such, the timer to stop the beep expires while your WHILE loop is running, but the code to stop the beep only gets executed when the CPU is idle. That's the case when the function containing the WHILE loop is finished. (There might be better and technical more accurate explanations.)

In case you need to write

display(1, hourFirstDigit);
    display(2, hourSecondDigit);

repeatedly then do this using a function. Start the function with var yolo=setInterval(...) and stop it with clearInterval(yolo). Don't use WHILE or FOR in such cases.

Regarding

pretty simple, right?

No, it's not simple. You need two function while one function with 3 lines would do.

Sun 2019.07.07

Good Morning @barry_b_benson,

Agreeing with what the other have stated, but pay attention to the point made about single threaded above.

If I understand correctly, I think this is what you are after:

In place of Line #3

setTimeout(function () {  buttonBoop(650);  }, 50);

https://www.espruino.com/Reference#l__global_setTimeout

This is untested BTW:


Understand that the single threaded nature of Javascript will allow the buzzer to sound until it times out, before the LEDs will be allowed to illuminate. But as the duration is only 50msec, I doubt any will notice.

EDIT: The more I think about it;

buttonBoop(650);

setTimeout(function () {  

  data();

  while (digitalRead(select) != 1) {
    display(1, hourFirstDigit);
    display(2, hourSecondDigit);
  }

  allClear();

}, 50);

Inside the setWatch() sound the buzzer, then after a short interval, run the rest of the desired events.

EDIT: (much later) See #7 below

Introducing setTimeout for the hour display makes the code hard to understand. Why do you want to keep the WHILE loop that is making things complicated?

@maze1980, my belief is that @barry_b_benson desires to have the display illuminated for the entire duration the button is held down and not just a simple timeout after a button press as your simplicity implies. Yes it is blocking, but what else in this case would want to interrupt? As you suggest, If the LEDs are turned on, then a unique new event (another button press perhaps) will be required to turn them off. I'll defer to his response for confirmation.

Maybe this is what you had in mind??   Untested

var intervalID = {};
function ct() { clearTimeout( intervalID ); }
// Easier to type ct() than entire line

 
var delay = 5000;    // msec  Turn off in case button remains stuck down
var isButtonDown = false;  // Not used here but might be needed elsewhere

function startTimer() {

  isButtonDown = true;
  
  hoursDisplay();    // Turn on as code inside func() occurs after timeout

  intervalID = setTimeout(function () {

    clearDisplay();
  
  }, delay);
}

function stopTimer() {
  isButtonDown = false;
  ct();
  clearDisplay();
}

function clearDisplay() {
  display(1, clear);
  display(2, clear);
}

function hoursDisplay() {
  display(1, hourFirstDigit);
  display(2, hourSecondDigit);
}


setWatch(function(e) {
  stopTimer();
}, select, {repeat: true, debounce: 50, edge: "rising"});

As said, make it a deterministic state machine, and you get rid of all the problems... and you also do not need a while or other loop to wait until something happens... this could actually make Espriuno irresponsive - freeze - because the loop holds on to the thread... and any other event will not be serve by Espruiono ('s JavaScript engine). ...loops are for arduinians... (event driven, 'open / logical' loops is to go for with Espruino).

Sun 2019.07.07

I'll go out on a limb here for @barry_b_benson, myself and others @allObjects, as I/we may not have experience with state machines as you pointed out in #2 above. Although I/we get the pseudo code as presented, how might one code that suggestion?

EDIT: Isn't that what is done in #7 - How to improve?

ref: "In every state there is defined behavior which will only be executed when the object is in that state"
ref: https://guide.freecodecamp.org/software-engineering/design-patterns/finite-state-machine/

You don't need state machines for this simple case.

@Robin: Refering to post #1 and #4, to replace WHILE and FOR with setInterval/setTimeout/clearInterval, use this code when the button is pressed (first setWatch function, line 1) instead of WHILE:

yolo = setInterval(function() {  //the watch displays the hour
      display(1, hourFirstDigit);
      display(2, hourSecondDigit);    }, 50); //with 50ms if may flicker

When the button is released (second setWatch function , line 11) call clearInterval(yolo) and allClear(). Then show the minutes again with yolo = setInterval(), and use setTimeout to clear that interval after 300ms with clearInterval(yolo) again insted of FOR.

If the displays flickers at 50ms reduce the interval to maybe 25 or 20ms.

Note: Although we don't have the 'actual' seven segment wiring diagram, I'm inferring each segment is connected to a physical pin.

IMO @maze1980, it doesn't make sense (to me anyway) to continue to hammer on the outputs using digitalWrite() every 50msec, as just a one time write should be sufficient, unless of course the digits are in the state of changing.

For the next step of code optimization, @barry_b_benson it would be far more efficient to replace all those function calls with a simple array containing the 'on' state of each segment.

https://www.quackit.com/javascript/tutorial/two_dimensional_arrays.cfm

Although I tend to stay away from global arrays, in this case using your existing module, would be an easy addition.

Something like:   Untested

var arySegments = new Array(10);

// { A B C D E F G }  where G is middle segment - L91 inside link of post #1
// This works nice here with digits as element zero of arySegments[0] is zero
// See usage inside function display() below

// For Zero
var aryDigitZero = { 1, 1, 1, 1, 1, 1, 0 };
  . . . 
var aryDigitNine = { 1, 1, 1, 0, 0, 1, 1 };

// Load the master array with each segment lit definition
arySegments.push( aryDigitZero );
  . . . 
arySegments.push( aryDigitNine );


function display( value ) {
  
  for ( var idx = 0; idx < aryDigitZero.length;  idx++ ) {
    digitalWrite( arySegments[value], arySegments[value][idx] );
  }
}


// Usage to turn on a single digit

display( 8 );

Different example initializing two dimension arrays:

https://www.espruino.com/Morphing+Clock

When more than one digit needs to be displayed and they are multiplexed, then a repeated output is needed. To judge on that, @barry_b_benson would need to publish some schematics... should be done anyway... Looking through the code I see that there are two digits as expected... BUT: I noticed the while and the for... they should be converted to open loops... so that watch and other events can be served...

@barry_b_benson, to make this open loops, stick the loop body. In a function, then call the function from the main flow / line after initializing the loop condition(s), and in the loop body at the end append condition updates, such as counter increment, and then check condition, such as counter not yet reached max or button still pressed, then - on desired condition - call the function itself with(in) a timeout . This way your watch and other (timing) events will get served.

@Robin: If it makes sense or not we don't know. Might be doing some multiplexing of pins, due to limited IO pins. The key message is to use setInterval/setTimeout instead of while/for.