dlc wrote:
I have an odd problem and I'm hoping that by trying to explain it I can
solve it...
This should perhaps be on avr-chat rather than avr-gcc-list, since it is
not really a gcc issue.
Paulo (and others) have given you ideas that might help your problem - I
can give you a few pointers towards writing better code, especially for
the AVR. A few small changes could make your ISR many times smaller and
faster.
What I have is an timer overflow ISR that tics off every 10us. I've
coded it thusly:
// variables at the top (global)
//ISR variables, volatile by nature
volatile uint16_t t_10us=0;
volatile uint32_t t_1ms=0;
volatile uint16_t repeat=0;
volatile uint16_t match=0; // The single servo used
uint8_t servo = 150;
First off, ISR variables should *not* be volatile. "Volatile" tells the
compiler that the values might change (or be read) outside the code flow
it is handling at the time. So anything used only be the ISR should not
be volatile - "volatile" costs time and space.
On the other hand, variables that are used in both the main loop, and
ISRs, should normally be volatile. Thus "servo" should be volatile, as
should "t_1ms" (since it is read in WaitMS), but all the others should
not be volatile.
In fact, the missing "volatile" on servo could be part of the cause of
your problems - there is no reason for the compiler to update the
variable's memory just because you wrote "servo = i" - it could well
leave the update until after the for() loop in your main code.
Secondly, the AVR is an eight bit processor. Don't use 16-bit or 32-bit
data unless you really need it (or you're writing portable code, or
something). Using a 16-bit variable for a counter that maxes out at 100
is much less efficient. Just try changing your "volatile uint16_t
t_10us" to "static uint8_t t_10us" and look at the difference in the
generated assembly for the ISR. Go through your other variables, and
make sure they are of minimal size, are only volatile if they have to
be, and that they are "static" if they are not exported by this module.
One last thing - I don't know if it was just a matter of format mangling
by email clients, but the following line pair is hideously dangerous.
You are hereby sentenced to three days Python programming.
> if (match == servo)
> SPIN=0; //drop servo bit
The only sane way to write if() statements is to include the brackets on
every occasion, even if you have only one statement in the conditional.
It's sometimes appropriate to omit the brackets if your code is all on
a single line, and the it's a simple assignment (no risk of
complications with macros), if that makes your code clearer. But if you
are using two lines, use brackets every time.
mvh.,
David
// past initializations and such ...
SIGNAL(SIG_OUTPUT_COMPARE0)
/*
* 10 microsecond ISR
*/
{
t_10us++;
if (t_10us > 100)
{
t_1ms++; //1ms background clock
t_10us = 0;
repeat++;
if (repeat == 19)
{
repeat = 0;
SPIN = 1; //raise servo pin high
match = 0; //start servo timer
}
}
match++; //increment every 1
if (match == servo)
SPIN=0; //drop servo bit
}
Now for the weird part. This all above seems to work fine unless I
update "servo" too often. As seen below:
// More code goes by...
servo = 100;
for (i=100;i<215;i+=2)
{
servo = i;
// If these lines are used it seems to work, but with skipped beats
// printf("servo = %d\n\r",i):
// WaitMS(40);
// With this next line I just get the extremes, no intermediate steps
WaitMS(150);
if(PIR)
{
RIGHT_STATUS = 1;
break;
}
}
What happens is that I only get the servo to move to the extreme
locations, none of the intermediate ones when I am not using the
printf() routine. This suggests a timing issue, but at 9600 baud those
printf()'s aren't taking more than 10-12ms I'd guess, and its hardware
so they shouldn't block either. I'm baffled.
void WaitMS(uint16_t mswait)
/*
* WaitMS() will delay for the considered number of ms.
*/
{
uint32_t started = t_1ms + mswait;
while (t_1ms < started)
;
return;
}
While I'm typing this it comes to me that perhaps the assembly in the
ISR is not so compact as I would like and that it simply does not have
time to do what needs done - But 150ms? I'm not so sure. Can anyone
offer some suggestions why this code will not function properly? Any
ideas accepted, including "Dennis you ignorant <expletive deleted>...
In the mean time, I'm going to examine the assembler output...
many thanks,
DLC
_______________________________________________
AVR-GCC-list mailing list
[email protected]
http://lists.nongnu.org/mailman/listinfo/avr-gcc-list
