Minimal Blink fails with void yield()

Paul:
Did you consider this change that Frank pointed out in the hardware\teensy\avr\cores\teensy3\yield.cpp::yield() function:
Code:
	running = 0;
	EventResponder::runFromYield();
};

should seemingly be:
Code:
	EventResponder::runFromYield();
	running = 0;
};
 
Okay - looking again I see I saw that once - so worst case would having it re-enter yield somehow and protect itself - or somehow complete in the middle of yield and then get called right away again. Being experimental that is fluid I suppose ... it still seems yield() mustn't be the final home for that with yield() being 'weak'/optional - like in main() after calling yield(): if (first != nullptr) runFromYield();. Also yield() calling from delay() might be a nice use of time - but could extend delay()'s?

I only saw that looking to see that my idea for serialEvent#'s to disable themselves by a flag bit per Frank won't work - as it is never called if no port .available so would require a second paired bit set on any receive to actually have that work.
 
it still seems yield() mustn't be the final home for that with yield() being 'weak'/optional

I wouldn't use the word "optional". ;)

The idea is scheduler libraries or an RTOS can override yield. Or you can, for whatever reasons you have.

But "with great power comes great responsibility" applies here. If you don't like the idea of yield possibly taking time, you can remove it, but you will suffer the consequence of losing the benefits it provides. Today you only lose the pretty worthless serialEvent callbacks. Long-term, I'm planning to build USB host and networking services and probably many other compelling applications on top of EventResponder.

If you're designing a multitasking system like TeensyThreads or a port of FreeRTOS or ChibiOS, you almost certainly will want to do EventResponder calls differently. That's the main reason yield() is meant to be something you can override. Whether EventResponder is flexible enough for those systems to override is probably the most experimental part of its experimental status.

But at least yield() can be overridden by alternate scheduling systems. That is the reason it's a weak symbol. It wasn't made weak with the intention for ordinary programs would override it, at least not without implementing their own approach.

Also yield() calling from delay() might be a nice use of time - but could extend delay()'s?

Yes, if an event handler takes time to execute it can add to delay(). It can also add time to complete any blocking function, like Serial1.print() when the stuff you're sending can't completely fit into the transmit buffer.

In essence, this is a very lightweight cooperative system. Like all cooperative sharing systems, time taken for 1 task can stall others. Trade-offs in multitasking are a huge subject that's been discussed many times over. The idea is the core library will provide a default implementation that's as simple and lightweight as possible. My hope is more complex systems like RTOS libs can successfully override these defaults to provide API compatible functionality, so in the long term we can craft libraries that will (hopefully) be far more compatible with multitasking systems than today's status quo.

At least that's the general idea. Did I mention this stuff is still pretty experimental? I don't have every answer. I haven't been able to think of everything in advance. I didn't even consider the weak alias problem that caused this bug. But I'm trying the best I can to move Teensy (and hopefully eventually Arduino) forward towards better support for event-based concurrency that defaults to the simplest possible cooperative system and can be extended to integrate nicely with more powerful cooperative and preemptive threading systems.
 
Experimental - yes that was noted and acknowledged :) Sounds like a reasonable plan - helping make usable RTOS libs and conversely getting some benefit using otherwise lost time.

I noted 'optional' because today it is - this thread 2nd post noted user case where imported library killed yield() and sketch was DOA. I assumed it was an external lib problem and left it - until the other poster commented on code size and I made this minimal blink fail.

It's just change - change is hard ... without a WIKI.
 
In most cases that I saw or used my own version of yield, is due to fact that in previous versions it was doing nothing of value and added a reasonable amount of overhead. That is why I earlier tried building a version that hopefully in the majority of cases would end up simply testing one variable (the bitmask approach like Frank's) and then return... Which then adding a simple call for run from yield would not be noticed...
(Other thread(s) I mentioned, included: https://forum.pjrc.com/threads/4400...and-when-internal-functions-should-call-yield

Thinking of Serial objects, I still have the version of Serial that made all of the objects from one class and added the ability to add to buffer size... I have not played with it in awhile (July 30th), but was working then. I believe there has been at least one recent fix to Serial (9 bit on Serial1/2) in mainline code... Should I try to revive this? More details in the thread:
https://forum.pjrc.com/threads/45213-Teensyduino-1-38-Beta-2?p=149640#post149640
 
Please hold off the unified HardwareSerial class for now. The need isn't really great, since the inheritance allows any to be used for libraries that want a HardwareSerial class pointer or reference.

I'd like to first focus on replacing the available polling with event responder, and find a way to do the fault handler that doesn't force the linker to pull most of the code for unused ports. Let's do the major refactoring after those are working and stable.
 
Please hold off the unified HardwareSerial class for now. The need isn't really great, since the inheritance allows any to be used for libraries that want a HardwareSerial class pointer or reference.

I'd like to first focus on replacing the available polling with event responder, and find a way to do the fault handler that doesn't force the linker to pull most of the code for unused ports. Let's do the major refactoring after those are working and stable.

Understood - I mainly did it as to make it simpler to do things like add the API(s) to increase size of send and/or receive buffers. Did not like the idea have having to duplicate the changes in six different places...
Actually make that 7 as Serial6 on T3.5 is the same as 1,2,3,4,5 just uses different UART5, whereas on T3.6, it is completely different subsystem, it uses LPUART0.

The is tucked away in a branch on my fork, so can revisit anytime...

Not sure how you wish to do the SerialEventx functions using Event handler, so will probably watch on that part.

As for Overhead of current stuff, my thoughts earlier were along the line Frank mentioned, again don't know if this makes Sense or goes along the line you are wanting to go. But some of the steps I have thought about trying include:

a) Move all of the default (weak) serialEvent objects out of HardwareSerialX.cpp files and maybe into one simple file that only has these... So hopefully if for example user does not do anything with Serial3, there is no reference to it and as such none of the code is brought in..
So have one file like Serialevents.cpp that has all of these functions:
Code:
void serialEvent1() __attribute__((weak));
void serialEvent1() {}

Edit: Tried this and did not make any code/data size changes... At least in the one program I tried...

b) Have a bitmask that we use in yield to see if any of the Serial objects (that we are interested in) has any input available... Note I would start with hardware serial ports. Probably different for Serial object.
1) Would have a variable in each Serial object which has the appropriate bit to set in the ISR that receives data
2) Update the read method (and clear) to clear this bit when appropriate.
3) Then yield (can simply first test if any bit is set and if not do nothing), else see which bit(s) are set and call the appropriate serialEvent object.
4) Now change the default serialEvent objects to clear that field that the ISR uses to say there is an event, such that our default serialEvent object should only ever be called once...

Again if this sounds interesting, I think there may be a few of us willing to try it out and make a Pull Request.
 
Last edited:
Regarding fault handler - do you mean the one for the NMIs?
If yes, i have an idea.. will be very simple - but I have to try it this evening.
 
Regarding fault handler - do you mean the one for the NMIs?
If yes, i have an idea.. will be very simple - but I have to try it this evening.

Ok, no answer, so I assume the fault handler for the NMIs is meant.

My worlds shortest serial print in software :rolleyes::
Code:
//By Frank B.
#define DEBUG_BAUD (115200)

void _serialDebugPrint(const uint8_t pin, const char * s) {
  const unsigned long bit = F_CPU / DEBUG_BAUD;
  ARM_DEMCR |= ARM_DEMCR_TRCENA;
  ARM_DWT_CTRL |= ARM_DWT_CTRL_CYCCNTENA;
  pinMode(pin, OUTPUT);
  digitalWriteFast(pin, 1);
  
  while (*s) {
   uint16_t c = (0x100 | *s++) << 1; //Add start & stop bits
   [COLOR=#d3d3d3][I] noInterrupts(); //<-- not needed in NMI[/I][/COLOR]
    unsigned long t = ARM_DWT_CYCCNT;    
    for (int i = 1; i<11; i++) {
      digitalWriteFast(pin, (c & 1) );
      c = c >> 1;      
      while ( ARM_DWT_CYCCNT - t < (i * bit) ) {;}
    }
  [COLOR=#d3d3d3] [I] interrupts();//<-- not needed in NMI[/I][/COLOR]
  }
}
Works with any pin, up to 4500000 Bit/s with Teensy 3.2 @96MHz

If someone wants to test: Connect pin 0 and 2, and use this sketch:
Code:
//By Frank B.
#define DEBUG_BAUD (115200)

void _serialDebugPrint(const uint8_t pin, const char * s) {
  const unsigned long bit = F_CPU / DEBUG_BAUD;
  ARM_DEMCR |= ARM_DEMCR_TRCENA;
  ARM_DWT_CTRL |= ARM_DWT_CTRL_CYCCNTENA;
  pinMode(pin, OUTPUT);
  digitalWriteFast(pin, 1);
  
  while (*s) {
   uint16_t c = (0x100 | *s++) << 1;
    noInterrupts(); //<-- not needed in NMI
    unsigned long t = ARM_DWT_CYCCNT;    
    for (int i = 1; i<11; i++) {
      digitalWriteFast(pin, (c & 1) );
      c = c >> 1;      
      while ( ARM_DWT_CYCCNT - t < (i * bit) ) {;}
    }
    interrupts();//<-- not needed in NMI
  }
}

void setup() {
  while (!Serial);
  Serial1.begin(DEBUG_BAUD);
}


void loop() {
  _serialDebugPrint(2, "Twas brillig, and the slithy toves Did gyre and gimble");
  Serial.println();
  while (Serial1.available()) {
    Serial.print((char)(Serial1.read()));
  }
  delay(5000);
}
 
Last edited:
Regarding fault handler - do you mean the one for the NMIs?

No, I mean the one in the core library, which is named fault_isr. You can find it starting at line 73 in mk20dx128.c. The part specifically about the hardware serial ports starts at line 129.

Those 3 lines are one of the things that are forcing so much hardware serial code to always get linked, even when never used.


Ok, no answer, so I assume the fault handler for the NMIs is meant.

For just a little perspective, only 4 hours elapsed between your messages for "no answer".
 
For just a little perspective, only 4 hours elapsed between your messages for "no answer".
Which simply means, there are some of us that do nothing else than reading/posting on the PJRC forum. (I'm kidding, and I appreciate your dedication to the community)
 
When a program "crashes" by accessing memory that doesn't exist, or trying to write to read-only memory, this fault hander allows whatever they had previously sent with Serial.print, Serial1.print, Serial2.print, or Serial3.print to actually get transmitted.
 
When a program "crashes" by accessing memory that doesn't exist, or trying to write to read-only memory, this fault hander allows whatever they had previously sent with Serial.print, Serial1.print, Serial2.print, or Serial3.print to actually get transmitted.

That is certainly a matter of opinion. One could argue that it is better to stop everything immediately - printing the buffer after that can be confusing, too.
You have to think about whether it's worth spending time on this - or to just ignore the output-buffers.
 
Last edited:
When a program "crashes" by accessing memory that doesn't exist, or trying to write to read-only memory, this fault hander allows whatever they had previously sent with Serial.print, Serial1.print, Serial2.print, or Serial3.print to actually get transmitted.
I've used this technique many times and it really does work quite well with printing to the usb serial during faults. Until we get real debug capabilities this gives you some insight into what the hell caused the fault by seeing what's in those register. If the user wants, you could add more debug info tailored to your particular program for further analysis in the fault handler.
 
Yep that printing is for sure useful. But we're talking about printing previously filled serial buffers.
Edit: That's a difference ;)
 
So you still want the usb isr called just not the hardware serial ports, your message was not clear on that to me.
 
I must admit, doing it this way was something of a quick fix in the very early days of Teensy 3.0. Not getting all your previously printed data was causing people a lot of pain, including me. At the time, I had only ported a few dozen libs, lots of new hardware wasn't supported... and my inbox was filling up with LED video wall requests from people who had seen this project and wanted me to provide a way to do it on the new, much cheaper WS2811 LEDs.

Since then, we've lived with this (and not really getting the weak linking quite right) pulling in most of the hardware serial code, even when none of the ports are actually used. Somehow massively refactoring the hardware serial code always seems like a low priority time-sinking project, though the saved memory would be really nice for Teensy LC users.

Any redesign must preserve the current behavior. We're not going back to those very early days where people got tricked into thinking their code crashed long before it actually did, only because they couldn't see the stuff they has printed long before the crash. Sure, one could argue another way, but the current behavior has almost 5 years of use and people (especially me) are generally happy with it. The 2 reasons for change are reducing yield overhead and linking in less unused code.

I'm trying to balance openness to ideas with prioritizing my dev time. I've currently got 12 items on my high priority bugs & features list (and hundreds more on the low priority lists). I recently got the new iMXRT chip... it's sitting here on my desk feeling neglected. It's also looking like the counterfeit boards problem is getting worse, which is pretty much the last way I want to spend my dev time, but we've got to at least detect them and show a message if PJRC is going to survive. Somewhere in all that, perhaps we can get this very mature hardware serial code refactored? I hope you can see how I'm not so excited to pour much dev time into it....
 
Looks Like I missed adding in Serial4-6 to here ;)

I actually did not know that the code was doing this... So I have at times: narrowed down where I thought things were crashing or hanging and doing things like: Serial1.println("I am here"); Serial1.flush();
... As to make sure the message get flushed out... Still need to do this at times for handling cases where code hangs...

Looks interesting to add in the #if 0 code in for debugging. Need to look a little more careful, but would be nice to know which fault happened and address of where it happened...
 
I recently got the new iMXRT chip... it's sitting here on my desk feeling neglected.
My preference would be to hear that work is progressing with the new iMXRT chip, and not that functionality of the 'cores' SW has changed, especially as an ino level suggestion for such behaviour change was published (I'm doing it).
 
Not sure how you wish to do the SerialEventx functions using Event handler, so will probably watch on that part.

The basic idea is to put an EventResponder in every HardwareSerial object.

Inside begin(), something like this:

Code:
  event.attach(serial1Event1);

Perhaps if the weak symbol is done correctly (needs to be in the same .cpp file), a compile time check can be done, like this:

Code:
  if (serial1Event1) event.attach(serial1Event1);


Inside the interrupt when data is received, something like this:

Code:
  if (!event) event.triggerEvent();

This will add a little more CPU usage inside the interrupt. But in the common case where the event is already triggered as more bytes arrive quickly, the boolean check is an inline read of just a single bool.

It's also going to add some code size, for projects that don't use EventResponder at all.

But the hopeful benefit is we can get rid of the larger overhead in yield(). EventResponder's runFromYield() only runs 1 event maximum per call. In no events are pending, it is (hopefully) much faster than the approach we have now of checking all 4 or 7 things. Of course, it will also support much more than just serialEvent, as we start to make use of it in USB Host, networking, non-blocking I/O, pin change debouncing, etc...
 
Last edited:
Back
Top