delay_1ms() failing at low oscillator speeds

This issue has been filed after a recent discussion in the Jallib dicsussion 
list.
Here a summary:

Below the relevant parts for the 1 millisecond delay procedure:

-- calculate instruction execution time in 10ns units
const instruction_time = 400_000_000 / target_clock

-- Delays for n * 1 msec
procedure delay_1ms(word in n) is
   const _one_ms_delay = 1000 - ((14 * instruction_time) / 100)
   for n loop
      if (_one_ms_delay <= 1000) then ; check if delay is not negative
         _usec_delay(_one_ms_delay)
      else
         _usec_delay(1)
      end if
   end loop
end procedure

The conditional expression in the 'if' statement is always true because the 
value of
_one_ms_delay is always lower than 1000. The intention of this 'if' is to 
prevent
that a zero or negative value is passed to _usec_delay(). This may occur with 
very
low oscillator frequencies (approx. 50000Hz or lower).
Proposal: change the if statement into: 

   if (_one_ms_delay > 0) then  

But tests showed that even then the 'else' branch is never taken! For some yet
unknown reason the constant '_one_ms_delay' is apparently handled as unsigned 
integer
in the conditional expression (while it is supposed to be a 32 bits signed 
integer!).  
Proposal: Explicitly declare '_one_ms_delay' and 'instruction_time' as type 
'sdword'.
Tests showed that then the 'else' branch is correctly taken with oscillator
frequencies lower than 50 KHz.

Both proposed changes were tested with the following program:

-- ------------------------------------------------------
-- Title: Test of delay_1ms procedure with a 16f690 on internal oscillator
-- ------------------------------------------------------
include 16f690                      -- target PICmicro
pragma target OSC  INTOSC_NOCLKOUT  -- internal oscillator
pragma target WDT  disabled         -- no watchdog
pragma target MCLR external         -- reset externally
--
pragma target clock    31250        -- oscillator frequency
OSCCON_IRCF = 0b000                 -- set internal oscillator
--
enable_digital_io()                 -- make all pins digital I/O
--
alias   led      is pin_C0          -- pin with the led
pin_C0_direction =  output
--
const sdword instruction_time = 400_000_000 / target_clock    -- in 10ns units
const sdword _one_ms_delay = 1000 - ((14 * instruction_time) / 100)
--
forever loop
   led = !led                       -- flip
   if (_one_ms_delay > 0) then
      _usec_delay(_one_ms_delay)
   else
      _usec_delay(1)
   end if
end loop
--

Note: The delay_10us() procedure of the delay library has similar constructions 
as
the delay_1ms procedure and will probably have to be revised as well.
Original comment by robhamerling on 3 May 2010 at 8:10
Changed state: Accepted
Added labels: Component-Jal
sumikchakka / jallib

delay_1ms() failing at low oscillator speeds #126
