Search code examples
cembeddedbit-shiftusart

Bitshifting vs array indexing, which is more appropriate for usart interfaces on 32bit MCUs


I have an embedded project with a USART HAL. This USART can only transmit or receive 8 or 16 bits at a time (depending on the usart register I chose i.e. single/double in/out). Since it's a 32-bit MCU, I figured I might as well pass around 32-bit fields as (from what I have been lead to understand) this is a more efficient use of bits for the MPU. Same would apply for a 64-bit MPU i.e. pass around 64-bit integers. Perhaps that is misguided advice, or advice taken out of context.

With that in mind, I have packed the 8 bits into a 32-bit field via bit-shifting. I do this for both tx and rx on the usart.

The code for the 8-bit only register is as follows (the 16-bit register just has half the amount of rounds for bit-shifting):

int zg_usartTxdataWrite(USART_data*         MPI_buffer,
                        USART_frameconf*    MPI_config,
                        USART_error*        MPI_error)
{

MPI_error = NULL;

if(MPI_config != NULL){
    zg_usartFrameConfWrite(MPI_config);
}

HPI_usart_data.txdata = MPI_buffer->txdata;

    for (int i = 0; i < USART_TXDATA_LOOP; i++){
        if((USART_STATUS_TXC & usart->STATUS) > 0){
            usart->TXDATAX = (i == 0 ? (HPI_usart_data.txdata & USART_TXDATA_DATABITS) : (HPI_usart_data.txdata >> SINGLE_BYTE_SHIFT) & USART_TXDATA_DATABITS);
        }
        usart->IFC |= USART_STATUS_TXC;
    }
    return 0;
}

EDIT: RE-ENTERTING LOGIC OF ABOVE CODE WITH ADDED DEFINES FOR CLARITY OF TERNARY OPERATOR IMPLICIT PROMOTION PROBLEM DISCUSSED IN COMMENTS SECTION

(the HPI_usart and USART_data structs are the same just different levels, I have since removed the HPI_usart layer, but for the sake of this example I will leave it in)

#define USART_TXDATA_LOOP 4
#define SINGLE_BYTE_SHIFT 8

typedef struct HPI_USART_DATA{

   ...
   uint32_t txdata;
   ...

}HPI_usart

HPI_usart HPI_usart_data = {'\0'};

const uint8_t USART_TXDATA_DATABITS = 0xFF;

int zg_usartTxdataWrite(USART_data*         MPI_buffer,
                        USART_frameconf*    MPI_config,
                        USART_error*        MPI_error)
{

MPI_error = NULL;

if(MPI_config != NULL){
    zg_usartFrameConfWrite(MPI_config);
}

HPI_usart_data.txdata = MPI_buffer->txdata;

    for (int i = 0; i < USART_TXDATA_LOOP; i++){
        if((USART_STATUS_TXC & usart->STATUS) > 0){
            usart->TXDATAX = (i == 0 ? (HPI_usart_data.txdata & USART_TXDATA_DATABITS) : (HPI_usart_data.txdata >> SINGLE_BYTE_SHIFT) & USART_TXDATA_DATABITS);
        }
        usart->IFC |= USART_STATUS_TXC;
    }
    return 0;
}

However, I now realize that this is potentially causing more issues than it solves because I am essentially internally encoding these bits which then have to be decoded almost immediately when they are passed through to/from different data layers. I feel like it's a clever and sexy solution, but I'm now trying to solve a problem that I shouldn't have created in the first place. Like how to extract variable bit fields when there is an offset i.e. in gps nmea sentences where the first 8 bits might be one relevant field and then the rest are 32bit fields. So it ends up being like this:

32-bit array member 0:

 bits 24-31      bits 15-23          bits 8-15            bits 0-7

| 8-bit Value | 32-bit Value A, bits 24-31 | 32-bit Value A, bits 16-23 | 32-bit Value A, bits 8-15 |

32-bit array member 1:

 bits 24-31             bits 15-23           bits 8-15               bits 0-7

| 32-bit Value A, bits 0-7 | 32-bit Value B, bits 24-31 | 32-bit Value B, bits 16-23 | 32-bit Value B, bits 8-15 |

32-bit array member 2:

 bits 24-31        15-23 8-15 ...

| 32-bit Value B, bits 0-7 | etc... | .... | .... |

The above example requires manual decoding, which is fine I guess, but it's different for every nmea sentence and just feels more manual than programmatic.

My question is this: bitshifting vs array indexing, which is more appropriate?

Should I just have assigned each incoming/outgoing value to a 32-bit array member and then just index that way? I feel like that is the solution since it would not only make it easier to traverse the data on other layers, but I would be able to eliminate all this bit-shifting logic and then the only difference between an rx or tx function would be the direction the data is going.

It does mean a small rewrite of the interface and the resulting gps module layer, but that feels like less work and also a cheap lesson early on in my project.

Also any thoughts and general experience on this would be great.


Solution

  • Since it's a 32-bit MCU, I figured I might as well pass around 32-bit fields

    That's not really the programmer's call to make. Put the 8 or 16 bit variable in a struct. Let the compiler add padding if needed. Alternatively you can use uint_fast8_t and uint_fast16_t.

    My question is this: bitshifting vs array indexing, which is more appropriate?

    Array indexing is for accessing arrays. If you have an array, use it. If not, then don't.

    While it is possible to chew through larger chunks of data byte by byte, such code must be written much more carefully, to prevent running into various subtle type conversion and pointer aliasing bugs.

    In general, bit shifting is preferred when accessing data up to the CPU's word size, 32 bits in this case. It is fast and also portable, so that you don't have to take endianess in account. It is the preferred method of serialization/de-serialization of integers.