[Solved - Tone is now blocking] Updated Core, Serial out now appears to block

Post here first, or if you can't find a relevant section!
stevestrong
Posts: 1821
Joined: Mon Oct 19, 2015 12:06 am
Location: Munich, Germany

Re: [Tone is now blocking] Updated Core, Serial out now appears to block

Post by stevestrong » Thu Dec 07, 2017 2:52 pm

Ray, I was just thinking what happens if a previous tone was set to play for a specific duration, but before the duration is over, a new tone is being set.
Imho, this usecase is not covered by the official command description.
One could just blindly follow the description and break it, playing the new pitch.
However, this is not the point of giving a duration as parameter.
If i specify a duration, then i would expect this duration to be kept, whatever comes afterwards.

victor_pv
Posts: 1746
Joined: Mon Apr 27, 2015 12:12 pm

Re: [Solved - Tone is now blocking] Updated Core, Serial out now appears to block

Post by victor_pv » Thu Dec 07, 2017 3:55 pm

stevestrong wrote:
Thu Dec 07, 2017 10:50 am
OK, but then at least block at the beginning if a previous tone is playing on the same pin with a given duration.
According to the Arduino API, if you call a second time in the same pin while playing, it only changes the frequency, does not block.

playTone is a different function, perhaps the best thing to do is to add that, if the behaviour is different on that one.

From:
https://www.arduino.cc/reference/en/lan ... d-io/tone/
Only one tone can be generated at a time. If a tone is already playing on a different pin, the call to tone() will have no effect. If the tone is playing on the same pin, the call will set its frequency.

User avatar
mrburnette
Posts: 1885
Joined: Mon Apr 27, 2015 12:50 pm
Location: Greater Atlanta
Contact:

Re: [Solved - Tone is now blocking] Updated Core, Serial out now appears to block

Post by mrburnette » Thu Dec 07, 2017 5:44 pm

... and hence the danger. Two STM32duino experts debate what the official Arduino code is really doing (or suppose to do.)

Arduino documentation is voluminous... but specifics are often difficult to pin-down without actually analyzing the official core(s) ... I'm not sure that Arduino.cc is consistent across the different architectures.

Ray

victor_pv
Posts: 1746
Joined: Mon Apr 27, 2015 12:12 pm

Re: [Solved - Tone is now blocking] Updated Core, Serial out now appears to block

Post by victor_pv » Thu Dec 07, 2017 6:29 pm

I bet is not consistent, but in any case this one is documented, and Roger verified the behaviour.
Keeping the old style (nonblocking) is compatible with Arduino, with old leaflabs, and makes the best use of the MCU.

If one wants to block for the duration of a tone, just needs to add delay(xx); right after.
If it's going to be a loop playing a melody or whatever, then anyone can write a new function that takes the note and duration as parameter and calls both tone() and delay().

I agree with you that we should diverge from the official core or API when either:
-Is for better compatibility with libmaple
-Or to make better usage of the hardware

This case meets neither condition, so no reason to diverge. I would not add another parameter to the tone function either. That means adding more code inside, and more wasted cpu cycles and flash space.

Code: Select all

waitTone (frequency, duration){
    tone(frequency, duration);
    delay(duration);
}
Anyone who wants to block, add this in your sketch and use it instead of tone, problem solved.

stevestrong
Posts: 1821
Joined: Mon Oct 19, 2015 12:06 am
Location: Munich, Germany

Re: [Solved - Tone is now blocking] Updated Core, Serial out now appears to block

Post by stevestrong » Thu Dec 07, 2017 6:41 pm

Victor, your example is pointing exactly at the issue: what would be the reason to pass a duration by calling tone(), if you anyway have to add a delay afterwards?
I would agree with you when calling tone without any duration:

Code: Select all

    tone(tonePin, 200);
    delay(duration);
In my view this should be equal to:

Code: Select all

    tone(tonePin, 200, duration);
Now, what happens if you have a series of tone calls with duration?
According to the official API, the next tone() should have immediate effect, but it will not do what I expect, namely to finish the first tone having first duration, then start the next tone having second duration:

Code: Select all

    tone(tonePin, 200, duration);
    tone(tonePin, 400, 2*duration); // this should not break the previous tone
Wouldn't this make sense?
Similar discussion here: https://github.com/adafruit/Adafruit_Ci ... /issues/17.

I would be really interested to see how many users are using this tone function by passing the duration, and what kind of behavior do they expect from that.
I had an application in which this was really an issue. That is why I submitted a PR to Roger that time.

victor_pv
Posts: 1746
Joined: Mon Apr 27, 2015 12:12 pm

Re: [Solved - Tone is now blocking] Updated Core, Serial out now appears to block

Post by victor_pv » Thu Dec 07, 2017 7:45 pm

Problem is, tone() is often used to signal alarms, key presses, etc.
You want the tone to last long enough so the user can hear the beep, but blocking you code for 200 mS every time a user rotate the encoder input or presses a key is a big waste of CPU cycles.
So you send the command to beep and you go on your business without having to block or call noTone after X mS again.

User avatar
RogerClark
Posts: 7478
Joined: Mon Apr 27, 2015 10:36 am
Location: Melbourne, Australia
Contact:

Re: [Solved - Tone is now blocking] Updated Core, Serial out now appears to block

Post by RogerClark » Thu Dec 07, 2017 8:08 pm

For something as simple as tone() we need to mimic what the AVR board would do, I,e not block.

The only thing worth debating would be whether we allow different tones on different pins.
On the AVR they can only play a tone on 1 pin at a time, but I presume this was because of a AVR limitation, not because the Arduno API developers specificly wanted that operation.


But we should get the basics the same, and hence should change it to work like the AVR does, and just get 1 pin operation working.

gilhad
Posts: 18
Joined: Thu Oct 26, 2017 11:45 pm

Re: [Solved - Tone is now blocking] Updated Core, Serial out now appears to block

Post by gilhad » Sun Dec 10, 2017 3:16 am

stevestrong wrote:
Thu Dec 07, 2017 6:41 pm
I would be really interested to see how many users are using this tone function by passing the duration, and what kind of behavior do they expect from that.

Here is some snipet from one of my old Arduino programs - it beeps on start, while playing some light show on indicators. Pretty straighforward - set a sound to play some time, while I am demonstrating lights are working and unit is "booting". (Show(x) shows x as binary number on LEDs).

tone(pin, frequency, duration) is NOT blocking, otherwise it would look terribly.

IMHO it is typical usage of tone as otherwise it would not be possible to make such effects easily (which is Arduino all about). It is easy this way and it is also easy to make tone "blocking" by adding delay() after it.

Code: Select all

	for ( int i=0;i<6;i++){
		pinMode(leds[i], OUTPUT);
		digitalWrite(leds[i], LOW);
	};
	pinMode(pOut,OUTPUT);
	pinMode(pPiezzo,OUTPUT);
	digitalWrite(pOut,LOW);
	digitalWrite(pPiezzo,LOW);
	// }}}
	// {{{ ===========================  Make light show  =============================================
	tone(pPiezzo,440,600); // beep :)
	for (int i=0;i<63;i++){ Show(i); delay(10);};
	tone(pPiezzo,660,600); // beep :)
	for ( int i=0;i<6;i++){
		digitalWrite(leds[i], HIGH);
		delay(100);
	};
	tone(pPiezzo,880,600); // beep :)
	for ( int i=0;i<6;i++){
		digitalWrite(leds[i], LOW);
		delay(100);
	};
	tone(pPiezzo,880,600); // beep :)
	for (int i=0;i<63;i++){ Show(i); delay(10);};
	tone(pPiezzo,440,20);
	delay(20);
	// }}}
	// {{{ go serious


User avatar
RogerClark
Posts: 7478
Joined: Mon Apr 27, 2015 10:36 am
Location: Melbourne, Australia
Contact:

Re: [Solved - Tone is now blocking] Updated Core, Serial out now appears to block

Post by RogerClark » Sun Dec 10, 2017 5:26 am

Yes.

We have established that a Pull Request changed the functionality, and that was incorrect.

We now need to resolve this issue, however its not the only problem on the core at the moment, so I'm investigating one problem at a time

User avatar
RogerClark
Posts: 7478
Joined: Mon Apr 27, 2015 10:36 am
Location: Melbourne, Australia
Contact:

Re: [Solved - Tone is now blocking] Updated Core, Serial out now appears to block

Post by RogerClark » Sun Dec 10, 2017 5:38 am

It looks like a partial fix is to remove line 108 added in this PR

https://github.com/rogerclarkmelbourne/ ... 5c4e8#L109

I will need to connect a piezo speaker to a board and test to see if that change would be a total fix or if there are side effects

Post Reply