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

danSTM
Wed Dec 06, 2017 5:47 pm
Greetings,

I’m wondering if anyone has seen this issue. I was using the master branch (core was downloaded on 7/23/17). After updating from the master today, I noticed that my Serial.prints now take 100x longer or so. They appear to block and are not buffered.

So some change between the end of July and now seemed to cause this. Am I missing something?

Thanks!


mrburnette
Wed Dec 06, 2017 6:07 pm
Not to be rude, but you could diff the two images if you backed up the old one. Or, you could look at Roger’s change-log on github.

I remember some changes in serial, but nothing that will not be shown from this query:
https://www.google.com/search?q=serial+ … 2duino.com

Ray


danSTM
Wed Dec 06, 2017 8:07 pm
I was throw off by the fact I use a tone during my elapsed time benchmark. There was a change to the tone.cpp https://github.com/rogerclarkmelbourne/ … e.cpp#L189 that now blocks until the tone completes. This wasn’t the case before.

Glad I found it. Cheers!


RogerClark
Wed Dec 06, 2017 9:19 pm
Changes were made to tone to bring them inline with the official Arduino API

victor_pv
Thu Dec 07, 2017 2:24 am
Roger, I haven’t tried tone in an Arduino board, but from the examples and the code itself, it seems like tone should not block.

RogerClark
Thu Dec 07, 2017 3:37 am
I’ll have to take another look, but I thought there were 2 modes of operation, i.e blocking and non-blocking

i,.e if you specify a duration, it blocks and if you don’t it just returns and keeps the tone going


stevestrong
Thu Dec 07, 2017 10:29 am
[RogerClark – Thu Dec 07, 2017 3:37 am] –
i,.e if you specify a duration, it blocks and if you don’t it just returns and keeps the tone going

Yea, that was my interpretation, too (it was my PR which caused the change 8-) )
I found this having the same context: https://github.com/adafruit/Adafruit_Ci … /issues/17
Accordingly, we could eventually add a fourth parameter for tone() to specify blocking or not blocking, so that backwards compatibility is kept.

Or, we can check at the beginning of tone() whether a previous tone is still playing on the same pin, and only block in this case.


RogerClark
Thu Dec 07, 2017 10:48 am
Steve

I just tried it on a Uno, and it doesnt seem to block

So we’ll need to change it back to non-blocking


stevestrong
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.

mrburnette
Thu Dec 07, 2017 1:31 pm
Respectfully …

This discussion is why I am generally opposed to hacking around on the core files without a clear technical mandate as to the change … interpretation of what is and what is not being done in the Arduino.cc code base. Then, there is the history of the Leaflabs core that I feel needs to be taken into account as there is a mountain of forum materials over on that site that our efforts here will affect. This concern was voiced yesterday by myself in another post.

There was a time in the past where ever member of this forum was capable of hacking the core to satisfy their particular desires. We have this expertise today throughout only a portion of our user community; many users are novices with ArduinoIDE and Arduino’ish hardware. Every day I read some post that simply should not have been posted because the answer is easily obtained with a search or with a test script.

In my thinking, it is not a critical concern to follow the Arduino.cc methodology exactly – provided that the WiKi reflects the changes and that we offer a work-around for our code base to mimic the Arduino.cc code base, whenever possible. In this regard, I believe that the STM32 silicon is significantly different such that a perfect 1::1 correspondence is not necessary.

Why should an advanced architecture even have blocking commands? We older programmers articulate the evils of blocking – why on earth would we purposefully create a blocking command in the core?

Core names for classes can be different from the Arduino official name. Just document same. If someone does not like it, they can hack the core and change the class names and accept responsibility for their stuff. Or, they can inherit from the core class and provide the class a new name. There is simply no justification to changing something just to be changing a name … who know what ramifications will occur?

Adding parameters to commands is a sensible solution but still must be documented in the WiKi… we cannot get away from doing the paperwork. The additional parameters must be implemented such that the old behavior is the default without the parameter being specified and the new behavior implemented only when the additional parameter is specified.

I have a lots of other opinions about “stuff” being proposed here … but this is not the time or place to expand on my opinions.

Ray

The Official Tone command from the .cc site:
Generates a square wave of the specified frequency (and 50% duty cycle) on a pin. A duration can be specified, otherwise the wave continues until a call to noTone(). The pin can be connected to a piezo buzzer or other speaker to play tones.

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.

Use of the tone() function will interfere with PWM output on pins 3 and 11 (on boards other than the Mega).

It is not possible to generate tones lower than 31Hz. For technical details, see Brett Hagman’s notes.

The official description of noTone:
Stops the generation of a square wave triggered by tone(). Has no effect if no tone is being generated.
If you want to play different pitches on multiple pins, you need to call noTone() on one pin before calling tone() on the next pin.

Maple documentation from the static Leaflabs.com site:
Generates a square wave of the specified frequency (and 50% duty cycle) on a pin. A duration can be specified, otherwise the wave continues until a call to noTone(). The pin can be connected to a piezo buzzer or other speaker to play tones.

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.

Use of the tone() function will interfere with PWM output on pins 3 and 11 (on boards other than the Mega).

NOTE: if you want to play different pitches on multiple pins, you need to call noTone() on one pin before calling tone() on the next pin.


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
Thu Dec 07, 2017 3:55 pm
[stevestrong – 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.


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
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.

waitTone (frequency, duration){
tone(frequency, duration);
delay(duration);
}


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:
tone(tonePin, 200);
delay(duration);

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.

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
Sun Dec 10, 2017 3:16 am
[stevestrong – 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.

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


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


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


RogerClark
Sun Dec 10, 2017 6:10 am
I’ve removed the line that blocks, and have done some timing tests, compared with an Arduino Uno, and the timings of my test program now seem identical on the Uno and the Maple mini.

Please update your local copy with the latest version and let me know if this resolves the problem


Leave a Reply

Your email address will not be published. Required fields are marked *