C 'desktop' programmer needs advice on how to code embedded C on a micro controller.

On 2007-04-08, David M. Palmer <dmpalmer@email.com> wrote:

> And sometimes the compiler is too smart for its own good. 
> Hypothetically:
>
>      register int16 foo=memloc;
>      foo++;
>      memloc = foo;
>
> gets optimized to 
>      memloc++;
>
> which gets optimized to
>   incr memloc_lo;
>   skip_if_no_carry;
>   incr memloc_hi;
>
> which usually saves a memory fetch and store.

No, the compiler is not too smart for its own good. It
generated correct code.

That's a case of the programmer not being smart enough.

if memloc was declared volatile as it should have been, the
compiler would have generated the code the programmer wanted.

-- 
Grant Edwards                   grante             Yow!  Just imagine you're
                                  at               entering a state-of-the-art
                               visi.com            CAR WASH!!

rickman wrote:
> 
> David did not post any of the IRQ code here, but it should include an
> overflow check.

So what do you do if there is an overflow? Its useful to have a flag 
stored somewhere noting the situation occurred but the situation is rare 
that one can do anything about it in fielded system.

> You should also check the overflow flag from the IRQ routine and if
> you are not certain of the adequacy of the buffer size, you may want
> to include a variable to store the max buffer space used.

Agree that is useful debugging information and one should do it but not 
much one can do about it after a system has shipped.

> Maybe this is a nit, but depending on the details of the message, you
> may need to recognize that you have not found a match and need to read
> to the end of the message before starting to scan for the next start
> of message.

I'm thinking he doesn't expect to understand everything coming from the 
GPS and only wants particular message(s). So an "I didn't understand 
that" flag might not be of any use.

> They also did everything in the interrupt code so that the idle loop
> was just that, a loop doing *nothing*!

I have seen systems coded like that. There are some schools of thought 
saying that is The One Right Way to do "embedded" and in doing so it 
becomes a Real Time System. The thought is that the system timing is now 
deterministic. I disagree and spent a lot of time disagreeing with 
management as to whether a legacy system actually worked as advertised. 
It did not because it was often deaf to new instructions immediately 
after receiving an instruction. It tried to keep time by counting 
interrupts, and the time had to be fudged with a correction factor 
because a command issued during interrupt almost always ran long and 
missed the next 10 or so interrupts.

David M. Palmer wrote:
> In article <28b79$4618394d$d8ba94f9$19383@KNOLOGY.NET>, David Kelly
> <n4hhe@yahoo.com> wrote:
> 
>> Don't modify the tail index in IRQ, don't modify the head index outside 
>> of IRQ. With that simple rule there is no need to protect either 
>> variable by disabling IRQs. Remember the head index can change at any 
>> time. In my example code the head should have been declared volatile.
> 
> 
> Also, the changes to the tail pointer must be 'atomic'.

No, it doesn't, not with the rules I layed out. The tail only changes 
outside of interrupt and is never used inside interrupt. There is no 
need for it to be atomic.

The head changes inside of interrupt and never changed outside. So it 
too is protected but probably should be volatile. The test was for 
equality between head and tail. If an interrupt occurred during the test 
and head was partially changed the equality test would still be valid. 
Might not indicate a new character is present this time but thats no 
loss, it will be picked up the next time. If head was changing and not 
equal to tail then there must be a new character present no matter the 
head value used for comparison was not perfectly accurate.

In olden days of m68k pointers were very efficient ways of managing 
buffers. In modern times I find an index is never any worse in speed or 
efficiency over a pointer. Especially if the buffer location is hard 
coded at link time.

On Apr 8, 7:15 pm, David Kelly <n...@yahoo.com> wrote:
> rickman wrote:
>
> > David did not post any of the IRQ code here, but it should include an
> > overflow check.
>
> So what do you do if there is an overflow? Its useful to have a flag
> stored somewhere noting the situation occurred but the situation is rare
> that one can do anything about it in fielded system.

That is exactly the point.  If the system crashes, you need to know
why.  If the overflow is just a matter of a data hiccup, then you can
indicate this by some means so the system does not use corrupted
data.  If it is likely to be fatal to the operation, then you can trap
it and halt the system in a way that you can tell what caused it.
Regardless, the last thing would would want is for the system to run
on without even knowing that there was a problem.

Better would be a size variable that tracks the max usage.  Then in
the debug stages of your project you can tell if you have sized it
with adequate headroom.


> > You should also check the overflow flag from the IRQ routine and if
> > you are not certain of the adequacy of the buffer size, you may want
> > to include a variable to store the max buffer space used.
>
> Agree that is useful debugging information and one should do it but not
> much one can do about it after a system has shipped.

But that is the point, you need to iron out any wrinkles before the
system ships.  Even after, there are often ways of reporting errors
that can make it back to the vendor.  My router has any number of
software flaws and it would have been nice it they had a way to report
the nature of the flaw.  I expect most of them are memory leaks and
they could be traced and fixed.  But that particular market uses a
"disposable" mindset and bugs are fixed by selling new units.  Many
embedded developers can't use that method since the customer goes away
if you have too many bugs.


> > Maybe this is a nit, but depending on the details of the message, you
> > may need to recognize that you have not found a match and need to read
> > to the end of the message before starting to scan for the next start
> > of message.
>
> I'm thinking he doesn't expect to understand everything coming from the
> GPS and only wants particular message(s). So an "I didn't understand
> that" flag might not be of any use.

I'm not saying the rest of the messager needs to be parsed, but
depending on the particulars of the message format, it may not work
well to start looking for the start of a message in the middle of the
last one.  There may be false hits that way.  But I don't know the
details of the message format, so I can't tell which works.  But
looking for the CR/LF or whatever terminator is used is sure to work.


> > They also did everything in the interrupt code so that the idle loop
> > was just that, a loop doing *nothing*!
>
> I have seen systems coded like that. There are some schools of thought
> saying that is The One Right Way to do "embedded" and in doing so it
> becomes a Real Time System. The thought is that the system timing is now
> deterministic. I disagree and spent a lot of time disagreeing with
> management as to whether a legacy system actually worked as advertised.
> It did not because it was often deaf to new instructions immediately
> after receiving an instruction. It tried to keep time by counting
> interrupts, and the time had to be fudged with a correction factor
> because a command issued during interrupt almost always ran long and
> missed the next 10 or so interrupts.

Using interrupts that way *can* work, but you have to understand it in
detail to make sure.  Then it can be harder to modify as the
requirements change or grow or new features are added.  By keeping the
int handlers short and using a properly structured control loop,
nearly any combination of timings can be handled and it can be much
easier to modify.

On Apr 8, 1:22 pm, "David M. Palmer" <dmpal...@email.com> wrote:
> In article <1176040035.163208.234...@n59g2000hsh.googlegroups.com>,
>
> rickman <gnu...@gmail.com> wrote:
> > On Apr 7, 10:52 pm, "David M. Palmer" <dmpal...@email.com> wrote:
> > > Also, the changes to the tail pointer must be 'atomic'.
> > So instead of incrementing a multiple
> > cell value in place, you can just copy to another variable (like a
> > register), increment it and copy it back.  But you do have to make
> > sure the copy back *is* an atomic operation.  Even on 8 bit MCUs, a 16
> > bit write is often a single instruction.
>
> And sometimes it's not.  (e.g. the AVR doesn't have a 16-bit store
> instruction.)
>
> And sometimes the compiler is too smart for its own good.
> Hypothetically:
>
>      register int16 foo=memloc;
>      foo++;
>      memloc = foo;
>
> gets optimized to
>      memloc++;
>
> which gets optimized to
>   incr memloc_lo;
>   skip_if_no_carry;
>   incr memloc_hi;
>
> which usually saves a memory fetch and store.

Which means you need to inspect the assembly produced and, if
required, use inline assembly.  If this is not a match for your
processor, then clearly you can't use it.  But if your processor has a
16 bit, atomic write, then you should be good to go.  Using interrupt
disables and enables is not very portable and has no advantages over
this method.  But there is more than one way to skin a cat...  my cat
suddenly left the room, I wonder why?

David M. Palmer wrote:
> In article <28b79$4618394d$d8ba94f9$19383@KNOLOGY.NET>, David Kelly
> <n4hhe@yahoo.com> wrote:
> 
>> Don't modify the tail index in IRQ, don't modify the head index outside 
>> of IRQ. With that simple rule there is no need to protect either 
>> variable by disabling IRQs. Remember the head index can change at any 
>> time. In my example code the head should have been declared volatile.
> 
> 
> Also, the changes to the tail pointer must be 'atomic'.
> 
> (I haven't read this whole thread, so you may have mentioned that.)
> 
> If you have an 8-bit processor modifying a 16-bit tail pointer a byte
> at a time, then if an interrupt can slide in between a) incrementing
> the low-byte from 0xff to 0x00, and b) incrementing the high byte with
> the carry, then you will have a bug which is VERY hard to find, but
> WILL bite you at the most inopportune time.

No. As stated in the original conditions the tail *index* is never 
modified during IRQ. In fact it is never used in IRQ. So there is no 
atomic question.

On Apr 8, 7:28 pm, David Kelly <n...@yahoo.com> wrote:
> David M. Palmer wrote:
> > In article <28b79$4618394d$d8ba94f9$19...@KNOLOGY.NET>, David Kelly
> > <n...@yahoo.com> wrote:
>
> >> Don't modify the tail index in IRQ, don't modify the head index outside
> >> of IRQ. With that simple rule there is no need to protect either
> >> variable by disabling IRQs. Remember the head index can change at any
> >> time. In my example code the head should have been declared volatile.
>
> > Also, the changes to the tail pointer must be 'atomic'.
>
> No, it doesn't, not with the rules I layed out. The tail only changes
> outside of interrupt and is never used inside interrupt. There is no
> need for it to be atomic.
>
> The head changes inside of interrupt and never changed outside. So it
> too is protected but probably should be volatile. The test was for
> equality between head and tail. If an interrupt occurred during the test
> and head was partially changed the equality test would still be valid.
> Might not indicate a new character is present this time but thats no
> loss, it will be picked up the next time. If head was changing and not
> equal to tail then there must be a new character present no matter the
> head value used for comparison was not perfectly accurate.
>
> In olden days of m68k pointers were very efficient ways of managing
> buffers. In modern times I find an index is never any worse in speed or
> efficiency over a pointer. Especially if the buffer location is hard
> coded at link time.

The above analysis is of the approach you have presented and it
appears to be correct.  If you want to have error checking, then I
think some of the operations have to be atomic or non-interruptable.

In article <131km4g7782c44@corp.supernews.com>, David Kelly
<n4hhe@Yahoo.com> wrote:

> David M. Palmer wrote:
> > In article <28b79$4618394d$d8ba94f9$19383@KNOLOGY.NET>, David Kelly
> > <n4hhe@yahoo.com> wrote:
> > 
> >> Don't modify the tail index in IRQ, don't modify the head index outside 
> >> of IRQ. With that simple rule there is no need to protect either 
> >> variable by disabling IRQs. Remember the head index can change at any 
> >> time. In my example code the head should have been declared volatile.
> > 
> > 
> > Also, the changes to the tail pointer must be 'atomic'.
> > 
> > (I haven't read this whole thread, so you may have mentioned that.)
> > 
> > If you have an 8-bit processor modifying a 16-bit tail pointer a byte
> > at a time, then if an interrupt can slide in between a) incrementing
> > the low-byte from 0xff to 0x00, and b) incrementing the high byte with
> > the carry, then you will have a bug which is VERY hard to find, but
> > WILL bite you at the most inopportune time.
> 
> No. As stated in the original conditions the tail *index* is never 
> modified during IRQ. In fact it is never used in IRQ. So there is no 
> atomic question.

For the receiving IRQ, the head index (where the incoming data is being
stored) is compared to the tail index (where the unprocessed data is
stored) to prevent the new data from overwriting unprocessed old data,
or at least to notice when it happens.  The tail index is modified by
the non-interrupt thread as it processes the data.

If the write to the tail pointer is in the middle of changing when the
interrupt occurs, the data can be silently corrupted.

-- 
David M. Palmer  dmpalmer@email.com (formerly @clark.net, @ematic.com)

David M. Palmer wrote:
> In article <131km4g7782c44@corp.supernews.com>, David Kelly
> <n4hhe@Yahoo.com> wrote:
> 
>> No. As stated in the original conditions the tail *index* is never 
>> modified during IRQ. In fact it is never used in IRQ. So there is no 
>> atomic question.
> 
> For the receiving IRQ, the head index (where the incoming data is being
> stored) is compared to the tail index (where the unprocessed data is
> stored) to prevent the new data from overwriting unprocessed old data,
> or at least to notice when it happens.  The tail index is modified by
> the non-interrupt thread as it processes the data.
> 
> If the write to the tail pointer is in the middle of changing when the
> interrupt occurs, the data can be silently corrupted.

Back to my original statement, that tail is never used in IRQ.

Wrote earlier in this thread that it is interesting to know when one has 
overflowed a buffer but that there isn't much one can do to recover in 
the field. Being a circular buffer an entire buffer full of data is lost 
but its not the same sort of thing as a buffer overflow security violation.

If one does not notice the data loss from an overflowed buffer then one 
might claim their application is robust and error tolerant. If the 
overflow causes your device to barf then one doesn't need debugging 
flags to indicate something isn't working. Flags might help find it.

Have seen others to go much effort to calculate the length of data in 
the buffer. Beyond debugging I have seen little need. If there is data 
waiting it needs to be processed no matter how much is waiting.

In article <131niap8cu13l63@corp.supernews.com>, David Kelly
<n4hhe@Yahoo.com> wrote:

> David M. Palmer wrote:
> > In article <131km4g7782c44@corp.supernews.com>, David Kelly
> > <n4hhe@Yahoo.com> wrote:
> > 
> >> No. As stated in the original conditions the tail *index* is never 
> >> modified during IRQ. In fact it is never used in IRQ. So there is no 
> >> atomic question.
> > 
> > For the receiving IRQ, the head index (where the incoming data is being
> > stored) is compared to the tail index (where the unprocessed data is
> > stored) to prevent the new data from overwriting unprocessed old data,
> > or at least to notice when it happens.  The tail index is modified by
> > the non-interrupt thread as it processes the data.
> > 
> > If the write to the tail pointer is in the middle of changing when the
> > interrupt occurs, the data can be silently corrupted.
> 
> Back to my original statement, that tail is never used in IRQ.

The tail is used in the IRQ, by comparing it to the head, to prevent
the data from being silently corrupted.

> If one does not notice the data loss from an overflowed buffer then one 
> might claim their application is robust and error tolerant. If the 
> overflow causes your device to barf then one doesn't need debugging 
> flags to indicate something isn't working. Flags might help find it.

If you have GPS NMEA strings coming in, and it is corrupted by the
buffer overflow, then either you have a problem or you weren't doing
anything important with the GPS information.  If you have a flag that
indicates that the data was truncated, then you can mitigate by, e.g.
extrapolating your new position based on previous data until you can
get back to the valid data.   If you don't, then you apply hard left
rudder because your aircraft carrier thinks it is 3000 feet over Des
Moines.  (Yes there should be sanity checking.  One good sanity check
is whether your shrink says that you're insane.)

It's useful for a pacemaker to know the difference between 'v-fib, time
to crank up the defibrillator' and 'data error--log the fault, schedule
maintenance, and wait for the next beat'.

> Have seen others to go much effort to calculate the length of data in 
> the buffer. Beyond debugging I have seen little need. If there is data 
> waiting it needs to be processed no matter how much is waiting.

Or it needs to mode switch to decrease the data rate.  Or it needs to
flush the stale data and get current.

-- 
David M. Palmer  dmpalmer@email.com (formerly @clark.net, @ematic.com)