4 consecutive bytes in buffer to unsigned long

Question

I wonder if there's a smarter way to get these 4 consecutive bytes from a buffer, concatenated in a unsigned long.

// the timestamp starts at byte 40 of the received packet and is four bytes,
// or two words, long. First, extract the two words:

unsigned long highWord = word(packetBuffer[40], packetBuffer[41]);
unsigned long lowWord = word(packetBuffer[42], packetBuffer[43]);
// combine the four bytes (two words) into a long integer
// this is NTP time (seconds since Jan 1 1900):
unsigned long secsSince1900 = highWord << 16 | lowWord;
Serial.print("Seconds since Jan 1 1900 = ");
Serial.println(secsSince1900);

Seems overkill to me, thinking these 4 bytes are already consecutive in the buffer.

Result should be unchanged i.e. having the value in a variable.

Answer 1

If they were in the same endian order as a long (little-endian on an Arduino, which they don't appear to be) you could just point to them:

uint32_t *l = (uint32_t *)&packetBuffer[40];

Then use *l to access the content as a long.

However since they look to be in a big-endian order it's not that simple. The simplest thing you can do is just re-combine them with bit-shifting:

uint32_t l = ((uint32_t)packetBuffer[40] << 24) | ((uint32_t)packetBuffer[41] << 16) | 
             ((uint32_t)packetBuffer[42] << 8) | (uint32_t)packetBuffer[43];

Answer 2

An alternative is to use union and struct, and let the compiler do the work:

static inline uint32_t bswap32(uint32_t x)
{
        union {
                uint32_t x;
                struct {
                        uint8_t a;
                        uint8_t b;
                        uint8_t c;
                        uint8_t d;
                } s;
        } in, out;
        in.x = x;
        out.s.a = in.s.d;
        out.s.b = in.s.c;
        out.s.c = in.s.b;
        out.s.d = in.s.a;
        return out.x;
} 

uint32_t sinceEpoch = bswap32(*(uint32_t*) &packetBuffer[40]);

Ref. http://lists.nongnu.org/archive/html/avr-gcc-list/2006-12/msg00076.html

A cleaner rewrite for the packet buffer would be:

static inline uint32_t bswap32(uint8_t* buf)
{
        union {
                uint32_t x;
                struct {
                        uint8_t a;
                        uint8_t b;
                        uint8_t c;
                        uint8_t d;
                } s;
        } in, out;
        out.s.a = buf[3];
        out.s.b = buf[2];
        out.s.c = buf[1];
        out.s.d = buf[0];
        return out.x;
} 

uint32_t sinceEpoch = bswap32(&packetBuffer[40]);

Here is an AVR machine code version:

inline uint32_t bswap32(uint32_t value)
{
  asm volatile("mov __tmp_reg__, %A0"   "\n\t"
           "mov %A0, %D0"       "\n\t"
           "mov %D0, __tmp_reg__"   "\n\t"
           "mov __tmp_reg__, %B0"   "\n\t"
           "mov %B0, %C0"       "\n\t"
           "mov %C0, __tmp_reg__"   "\n\t"
           : "=r" (value)
           : "0" (value)
           );
  return (value);
}

Ref. Cosa/Types.h, https://github.com/mikaelpatel/Cosa/blob/master/cores/cosa/Cosa/Types.h#L553, and Cosa-NTP/NTP.cpp, https://github.com/mikaelpatel/Cosa-NTP/blob/master/NTP.cpp#L56

Answer 3

I ended up doing this:

unsigned long ntp_time = 0;
for (byte i = 0; i < 4; i++){
  ntp_time = ntp_time << 8 | (byte) packetBuffer[40 + i];
}

This is what I would have used on a high level language and my first attempt before asking — which failed because it lacked casting to byte.

I still have to understand why it failed, being packetBuffer declared as

const byte *packetBuffer

which to me seems addressing bytes already...

Thanks to Majenko for help, his solution still cast all buffer bytes to 4 bytes, I prefer mine that keeps down to bytes.