Skip to content

Commit

Permalink
Fix incorrect handling of micros() overflow
Browse files Browse the repository at this point in the history
When micros() overflows, the LMIC tick value would make big jump back to
zero, which caused the library to grind to a halt (waiting for timer
values that never occurred). This fixes by detecting overflows and
virtually extending the tick counter with a few bits.
  • Loading branch information
matthijskooijman committed Feb 20, 2016
1 parent c32036d commit f19d2cb
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 3 deletions.
44 changes: 42 additions & 2 deletions src/hal/hal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,48 @@ static void hal_time_init () {
}

u4_t hal_ticks () {
// LMIC requires ticks to be 15.5μs - 100 μs long
return micros() / US_PER_OSTICK;
// Because micros() is scaled down in this function, micros() will
// overflow before the tick timer should, causing the tick timer to
// miss a significant part of its values if not corrected. To fix
// this, the "overflow" serves as an overflow area for the micros()
// counter. It consists of three parts:
// - The US_PER_OSTICK upper bits are effectively an extension for
// the micros() counter and are added to the result of this
// function.
// - The next bit overlaps with the most significant bit of
// micros(). This is used to detect micros() overflows.
// - The remaining bits are always zero.
//
// By comparing the overlapping bit with the corresponding bit in
// the micros() return value, overflows can be detected and the
// upper bits are incremented. This is done using some clever
// bitwise operations, to remove the need for comparisons and a
// jumps, which should result in efficient code. By avoiding shifts
// other than by multiples of 8 as much as possible, this is also
// efficient on AVR (which only has 1-bit shifts).
static uint8_t overflow = 0;

// Scaled down timestamp. The top US_PER_OSTICK_EXPONENT bits are 0,
// the others will be the lower bits of our return value.
uint32_t scaled = micros() >> US_PER_OSTICK_EXPONENT;
// Most significant byte of scaled
uint8_t msb = scaled >> 24;
// Mask pointing to the overlapping bit in msb and overflow.
const uint8_t mask = (1 << (7 - US_PER_OSTICK_EXPONENT));
// Update overflow. If the overlapping bit is different
// between overflow and msb, it is added to the stored value,
// so the overlapping bit becomes equal again and, if it changed
// from 1 to 0, the upper bits are incremented.
overflow += (msb ^ overflow) & mask;

// Return the scaled value with the upper bits of stored added. The
// overlapping bit will be equal and the lower bits will be 0, so
// bitwise or is a no-op for them.
return scaled | ((uint32_t)overflow << 24);

// 0 leads to correct, but overly complex code (it could just return
// micros() unmodified), 8 leaves no room for the overlapping bit.
static_assert(US_PER_OSTICK_EXPONENT > 0 && US_PER_OSTICK_EXPONENT < 8, "Invalid US_PER_OSTICK_EXPONENT value");
}

// Returns the number of ticks until time. Negative values indicate that
Expand Down
4 changes: 3 additions & 1 deletion src/lmic/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@
//#define CFG_sx1276_radio 1

// 16 μs per tick
#define US_PER_OSTICK 16
// LMIC requires ticks to be 15.5μs - 100 μs long
#define US_PER_OSTICK_EXPONENT 4
#define US_PER_OSTICK (1 << US_PER_OSTICK_EXPONENT)
#define OSTICKS_PER_SEC (1000000 / US_PER_OSTICK)

// hal.cpp sets up stdio so that a plain "printf" call prints to the
Expand Down

0 comments on commit f19d2cb

Please sign in to comment.