5

I have a 16 bit variable data, ie:

volatile uint16_t data;

I need to populate this value based on the contents of two 8 bit registers on an external sensor. These are accessed over I2C/TWI.

My TWI routine is async*, and has the signature:

bool twi_read_register(uint8_t sla, uint8_t reg, uint8_t *data, void (*callback)(void));

This reads the value of reg on sla into *data, then calls callback().

If I knew the uint16_t was arranged in memory as, say, MSB LSB, then I could do:

twi_read_register(SLA, REG_MSB, (uint8_t *)&data, NULL);
twi_read_register(SLA, REG_LSB, (uint8_t *)&data + 1, NULL);

However, I don't like baking endian dependence into my code. Is there a way to achieve this in an endian-independent way?

(side note: my actual workaround at the moment involves using a struct, ie:

typedef struct {
    uint8_t msb;
    uint8_t lsb;
} SensorReading;

but I'm curious if I could do it with a simple uint16_t)

EDIT

(* by async I mean split-phase, ie *data will be set at some point in the future, at which point the callee will be notifed via the callback function if requested)

Lundin
  • 195,001
  • 40
  • 254
  • 396
sapi
  • 9,944
  • 8
  • 41
  • 71
  • 1
    Why do you want to constrain yourself to pointer access to the 16 bit value? Most would do this with shift and add or or operators. However if you really want to do it with pointers, you could use macros which detect and compensate for the endian-ness. – Chris Stratton Apr 15 '13 at 14:27
  • No you cant...reliably. Use a mask and shift if you want reliable and portable code. – old_timer Apr 15 '13 at 20:09
  • actually the mask and shift doesnt work either, does it? – old_timer Apr 15 '13 at 20:10
  • @ChrisStratton - Masking and shifting simply doesn't work with split-phase calls, hence the question. It's no good masking and shifting after the calls to read, when the read function hasn't actually set the value yet. – sapi Apr 15 '13 at 21:54
  • Read into temporary values, then combine by masking & shift when you are notified the read is done. Even with pointers, you can't use the value between the time when you request the read and when you are notified that both are done, since you could have inconsistent high and low bytes if you catch it partway done. – Chris Stratton Apr 15 '13 at 22:09
  • If I read into temporary values, I have to allocate twice as much memory, which is a waste. Might as well use the struct / union trick in that case. – sapi Apr 16 '13 at 07:46
  • You are concerned about portability, but you are reading from an **i2c** controller? The **i2c** controller, the host, and the slave all define the swapping arrangement. See [htons](http://www.beej.us/guide/bgnet/output/html/multipage/htonsman.html). You need to conditionalized, based on the triple of actors. – artless noise Apr 16 '13 at 14:06

3 Answers3

4

Would the following not work?

uint8_t v1, v2;
twi_read_register(SLA, REG_MSB, &v1, NULL);
twi_read_register(SLA, REG_LSB, &v2, NULL);
data = ((uint16_t)v1<<8)|v2;

Or is data so volatile that the twi_read_register needs to write it. In that case I think you're stuck with endian dependent code.

As you pointed out below the data is indeed that volatile, because yet another device is reading it. So a memory mapped connection is established between two devices that may differ in endianness. This means you are stuck with endian dependent code.

You mention the struct as a workaround, but that is kind of a standard way of dealing with this.

#ifdef BIGENDIAN
typedef struct
{       uint8_t  msb, lsb;
} uint16_as_uint8_t;
#else
typedef struct
{       uint8_t  lsb, msb;
} uint16_as_uint8_t;
#endif

On top of that you could put a union

union
{       uint16_as_uint8_t  as8;
        uint16_t           as16;
};

Note that the latter is in violation of the C89 standard as it is your clear intention to write one field of the union and read from another, which results in an unspecified value. From off C99 this is (fortunately) supported. In C89 one would use pointer conversions through (char*) to do this in a portable way.

Note that the above may to seem hide the endianness in a portable way, structure packing may also differ from target to target and it might still break on some target. For the above example this is unlikely, but there are some bizarre targets around. What I'm trying to say is that it is probably not possible to program portable on this device level and it might be better to accept that and strive to hide all the details in a compact target interface, so changing one header file for the target would be enough to support it. The remainder of the code then can look target independent.

Bryan Olivier
  • 5,207
  • 2
  • 16
  • 18
  • 1
    Not sure why you're mentioning `volatile`, though. – Alexey Frunze Apr 15 '13 at 01:05
  • `twi_read_register` is asynchronous / split-phase, so `v1` and `v2` in your example won't be set until an indeterminate time in the future. `data` will have an undefined value. – sapi Apr 15 '13 at 01:06
  • I see. Then maybe you could do the combining into `uint16_t` once you know the values are ready. – Bryan Olivier Apr 15 '13 at 01:14
  • @AlexeyFrunze If `data` is really owned by some other device and it is out of control of the application when `data` is read, then we really need to pass its address to the `twi` device and we're stuck with endian dependent code. – Bryan Olivier Apr 15 '13 at 01:19
  • It's out of the application control, yes. I was wondering if there was some way to do it independent of endianness (I actually know the endianness of the target system, and it won't change, but I don't like writing code that depends on it, hence the struct workaround atm). – sapi Apr 15 '13 at 04:28
  • @sapi I've added some advice on hiding target dependences. Probably nothing new to you, but I felt it might be useful for others. – Bryan Olivier Apr 15 '13 at 10:21
  • Aliasing through a union is supported in C (6.5.2.3p3 and note 95). It's C++ where union aliasing is undefined behaviour. – ecatmur Apr 15 '13 at 10:29
  • @ecatmur I stand corrected, it shows how much of a dinosaur I am. I'd like to refer to [type punning](http://stackoverflow.com/questions/11639947/is-type-punning-through-a-union-unspecified-in-c99-and-has-it-become-specified) for more details. But it seems that from off C99 such unions will work. – Bryan Olivier Apr 15 '13 at 10:55
0

How about something like this?

uint16_t myValue = ...;
uint8_t LSB = (uint8_t)(myValue % 256);
uint8_t MSB = (uint8_t)(myValue / 256);
Alexandre Vinçon
  • 919
  • 5
  • 13
0
  volatile uint16_t data;  
  uint16_t v = 1;
  twi_read_register(SLA, REG_MSB, (uint8_t *)&data + *((uint8_t *)&v), NULL);
  twi_read_register(SLA, REG_LSB, (uint8_t *)&data + *((uint8_t *)&v + 1), NULL);
D Krueger
  • 2,446
  • 15
  • 12