That looks like a great start. I will read it in detail after my brain warms up from sleeping all day yesterday recovering from yet another inadvertent all-nighter the day before.
Somewhere in my archives is a long list of things which either bit me or I saw was potential trouble. I presented a short list in my recent class. I think I will start another thread to discuss those…
ISRs (and handling of multuple ISRs) may be an interesting topic, too. Although this maybe not be C-specific enough to include in a C-programming class.
Yes, especially covering the need to use the volatile keyword and then use atomic operations on those variables when dealing with variables that are changed within the ISR and accessed outside of it
volatile uint8_t gWDT_pool_start;
volatile uint8_t gWDT_pool_end;
volatile uint8_t gWDT_pool_count;
volatile uint32_t gWDT_entropy_pool[WDT_POOL_SIZE];
// This interrupt service routine is called every time the WDT interrupt is triggered.
// With the default configuration that is approximately once every 16ms, producing
// approximately two 32-bit integer values every second.
//
// The pool is implemented as an 8 value circular buffer
static void isr_hardware_neutral(uint8_t val)
{
gWDT_buffer[gWDT_buffer_position] = val;
gWDT_buffer_position++; // every time the WDT interrupt is triggered
if (gWDT_buffer_position >= gWDT_buffer_SIZE)
{
gWDT_pool_end = (gWDT_pool_start + gWDT_pool_count) % WDT_POOL_SIZE;
// The following code is an implementation of Jenkin's one at a time hash
// This hash function has had preliminary testing to verify that it
// produces reasonably uniform random results when using WDT jitter
// on a variety of Arduino platforms
for(gWDT_loop_counter = 0; gWDT_loop_counter < gWDT_buffer_SIZE; ++gWDT_loop_counter)
{
gWDT_entropy_pool[gWDT_pool_end] += gWDT_buffer[gWDT_loop_counter];
gWDT_entropy_pool[gWDT_pool_end] += (gWDT_entropy_pool[gWDT_pool_end] << 10);
gWDT_entropy_pool[gWDT_pool_end] ^= (gWDT_entropy_pool[gWDT_pool_end] >> 6);
}
gWDT_entropy_pool[gWDT_pool_end] += (gWDT_entropy_pool[gWDT_pool_end] << 3);
gWDT_entropy_pool[gWDT_pool_end] ^= (gWDT_entropy_pool[gWDT_pool_end] >> 11);
gWDT_entropy_pool[gWDT_pool_end] += (gWDT_entropy_pool[gWDT_pool_end] << 15);
gWDT_entropy_pool[gWDT_pool_end] = gWDT_entropy_pool[gWDT_pool_end];
gWDT_buffer_position = 0; // Start collecting the next 32 bytes of Timer 1 counts
if (gWDT_pool_count == WDT_POOL_SIZE) // The entropy pool is full
gWDT_pool_start = (gWDT_pool_start + 1) % WDT_POOL_SIZE;
else // Add another unsigned long (32 bits) to the entropy pool
++gWDT_pool_count;
}
}
Then to change those values outside of the ISR
ATOMIC_BLOCK(ATOMIC_RESTORESTATE)
{
retVal = gWDT_entropy_pool[gWDT_pool_start];
gWDT_pool_start = (gWDT_pool_start + 1) % WDT_POOL_SIZE;
--gWDT_pool_count;
}The Atomic Block directive is needed to ensure that the ISR doesn’t execute in the middle of changing the values that could also be modified within the ISR. This is needed anytime you are executing code that translates to more then a single machine language instruction.
That must be a pre-processor macro that saves/turns off/restores interrupts, as it is not part of the native C language.
It is likely to be in a header file he did not quote to create a critical section.
Most certainly. 
Wow!!! Got this thread suddenly active.
So how’bout how to create/write “pretty/elegant” code.
Had a discussion with a certain unnamed (ahem…) member on some of my embedded (arduino) pgms.
Went something like this:
You write ugly code.
It works
No, you think it works.
It does what I set out to accomplish.
It’s not reliable.
Hasn’t crashed. No random glitches, etc.
Not yet.
Not ever.
Global variables are evil.
It works.
“goto” is evil
OK - alternative?
Long sigh. Ugly code…
and so it goes.
At this stage I figure write the pgm and get it running reliably and then make it “pretty”.
Good coding techniques are an ongoing learning process.
How about code review/critique sessions??
…and that is why it was suggested that you post here…
Careful, that borders on religion…
No thanks. That is too much like work…
It is. It is contained in utils/atomic.h
The idea is that anytime you change (or even access) a variable that requires multiple machine language statements to be executed (like say working with two byte (integers) variables the access must be made to occur without the interrupt which alters that variable being triggered while you access it.
Think about what you get if you are ‘reading’ an integer (2-4 bytes) variable and the interrupt occurs during the read, say after the first of the two byte value is read, but before the second. At that point your code will be working with a variable that has one byte from one value and the second byte from the second value. This can lead to very hard to debug problems… DAMHIK
The really dangerous part is that the code without this construct will work MOST of the time…
…but that one other time ruins your day.
Use caution with this stuff because it tends to be platform specific. Declaring things ‘volatile’ in a multi-threaded environment is recommended for some platforms, but for other platforms it just kinda works…most of the time.
As the ‘unnamed member’ let me say that this first came up when you asked for help debugging some code that wasn’t working the way you wanted. What you showed me was a mass of spaghetti.
‘Good code’ is readable by someone else (or yourself in six months).
It should be in short manageable and readable sections.
Your variable names should have easily understood purpose/meaning.
Long functions/procedures/methods are to be avoided. It is best to break them up into more manageable size components.
Goto’s are uneeded in any modern language, other structures are available that allow for the creation of program logic without the need to resort to goto’s. Use of goto’s create code that is unreadable, convoluted, and often termed ‘spaghetti’. http://www.drdobbs.com/jvm/programming-with-reason-why-is-goto-bad/228200966 The ONLY time goto should be used is the JMP commands in assembly language (and then only if you are a masochist to be using assembly).
Global variables are easy. They also allow the programmer to avoid thinking about the problem at hand and structuring and compartmentalizing their code. The biggest drawback to globals is that you can be modifying one and WHILE you are modifying it, the same variable can be getting modified by another part of your program (thread/ISR/etc…)… and that leads to hard to debug problems. Unfortunately, on limited resource machines, like the AVR 328’s in most Arduino’s globals are a fact of life, but there use should be minimized.
Yes, I agree; however, I was thinking in context of the AVR series of Arduino’s which I think is what Bill is going to be focusing on in this class.
If your going to be covering the use of C (or other languages) in embedded systems, I suggest you introduce the use of Watch Dog Timers (internal or external) to provide an automatic restart method for those inevitable cases where a bug causes the device to lock up.
I vaguely remember a case where goto benchmarked significantly faster than any other way to do things. I think that’s why it keeps ending up in languages.
It is actually used in the Golang standard libraries too 
https://golang.org/src/math/gamma.go
Maybe ‘goto is USUALLY evil’ but that is true of most of the maligned programming constructs. I love this with the rule of thirds etc in photography etc.
The speed of a construct depends almost entirely upon the implementation of the complier/interpreter and not on the construct itself. On a good modern compiler, there is likely no difference in speed. machine languages all use JMP (or goto statements) so the code get compiled to those; however, a structured construct is much easier to read and understand, which is much more important when it comes around to maintaining the code.
Speed is also a mistake to worry about when first creating your code. When first developing the program your only goal should be for it to be easy for others to follow and maintain. When your code is working, is the time to look at ways to improve its speed.
BTW, Goto is evil because it is never actually needed when modern constructs are available, and it is always harder to follow the logic with goto’s. BTW, not familiar with Golang.
A goto may be the quickest at run time because of the way our processors currently function. A goto may be the quickest way to implement something the first time because the cost to maintain it is seldom considered.
Long term, it is bad news.
I would not say never and always. I consider it permissible to goto common error recovery/cleanup code at the bottom of a function when structured exception handling is not available or desired due to its overhead.
Oh, I know about mutexes and locks. I use them daily. 
I was just saying that the macro you were using isn’t part of the C language.