Search code examples
cembeddedwavmicrochippic18

PIC 18F45K42: How to combine 4 bytes to Int?


I am writing code for a PIC 18F45K42 to read .wav files from an HCSD card. I am using MPLAB X IDE v5.20 and XC v2.05. I am using FATFs library to read data off the card.

I can read data off the card and get good results until it comes to combining the 4 bytes representing the .wav file size.

#define BYTE_TO_BINARY_PATTERN "%c%c%c%c%c%c%c%c"
#define BYTE_TO_BINARY(byte)  \
(byte & 0x80 ? '1' : '0'), \
(byte & 0x40 ? '1' : '0'), \
(byte & 0x20 ? '1' : '0'), \
(byte & 0x10 ? '1' : '0'), \
(byte & 0x08 ? '1' : '0'), \
(byte & 0x04 ? '1' : '0'), \
(byte & 0x02 ? '1' : '0'), \
(byte & 0x01 ? '1' : '0') 

UINT actualLength;
UINT br;
char contents[44]; // Buffer
uint32_t file_size;
char result;

printf("Checking media\n\r");
if( SD_SPI_IsMediaPresent() == false)
{
    printf("No media\n\r");
    return;
}

 if (f_mount(&drive,"0:",1) == FR_OK)
    {
    if (f_open(&file, "P.WAV", FA_READ) == FR_OK){

        result = f_read(&file, contents, 44, &br);
        if ( result == FR_OK){
            printf("%c%c%c%c", contents[0],contents[1],contents[2],contents[3]);
            printf("\n\r");
            printf("contents 4-7: %u %u %u %u", contents[4],contents[5],contents[6],contents[7]);
            printf("\n\r");
            printf(""BYTE_TO_BINARY_PATTERN, BYTE_TO_BINARY(contents[7]));
            printf(""BYTE_TO_BINARY_PATTERN, BYTE_TO_BINARY(contents[6]));
            printf(""BYTE_TO_BINARY_PATTERN, BYTE_TO_BINARY(contents[5]));
            printf(""BYTE_TO_BINARY_PATTERN, BYTE_TO_BINARY(contents[4]));
            file_size = (contents[7] << 24) | (contents[6] << 16) | (contents[5] << 8) | contents[4];
            int kb = file_size/1024;
            printf("\n\r");
            printf("Size: %u kB: %u", file_size, kb);
            printf("\n\r");
// . . . OMITTED . . .

I am am printing the binary to reassure myself that the data written from the card is correct. The output I get is

Checking media
RIFF
contents 4-7: 4 122 18 0
00000000000100100111101000000100
Size: 31254 kB: 0

That binary gives me the correct number for the file size, 1210884, so the size I am getting by trying to get by combining the bytes is wrong. I presume this is is because I get the following complier warnings:

main.c:108:42: warning: shift count >= width of type [-Wshift-count-overflow]
            file_size = (contents[7] << 24) | (contents[6] << 16) | (contents[5] << 8) | contents[4];
                                     ^  ~~
main.c:108:64: warning: shift count >= width of type [-Wshift-count-overflow]
            file_size = (contents[7] << 24) | (contents[6] << 16) | (contents[5] << 8) | contents[4];

I have searched on these warnings and tried a number of recommended fixes, but so far I haven't found anything that works for this situation. I hasten to add that my C knowledge is limited, and I am not 100% sure what that bit shift expression is doing. My naive understanding is that the rightmost bit of contents[7] should be shifted to bit 24 of the 32bit integer, contents[6] rightmost bit to bit 16 etc. and somehow, probably because of some kind of data type issue, this is not happening properly. But of course I don't know, and I'm not sure what limitations there are in the Microchip XC compilier.

Short of recommending that I finally take a good course in C, can someone suggest what's going wrong and how to get the right value by assembling the 4 file size bytes properly.

Many thanks for looking.


Solution

  • Some basic things:

    • Never use the default types of C when programming embedded systems. Use stdint.h instead. The char type in particular is problematic, since it has implementation-defined signedness.
    • Doing arithmetic on small integer types is difficult on 8 bit MCUs. At a minimum, you must be aware of the Implicit type promotion rules.
    • 32 bit arithmetic is dreadfully slow on a PIC and should be avoided when possible. Probably not an option in this specific case.
    • PIC, being an 8-bitter, has 16 bit int.

    With that cleared up, we can note that each of these shifts is wrong:

    file_size = (contents[7] << 24) | (contents[6] << 16) | (contents[5] << 8) | contents[4];
    

    contents[i] is of type char (or uint8_t if you fix the code to the appropriate type). This is a small integer type. As explained in the link above, it will get implicitly promoted to int through integer promotion. This is why the compiler is giving you a warning. You need to cast each operand to uint32 before shifting.

    file_size = ( ((uint32_t)contents[7] & 0xFF) << 24) | 
                ( ((uint32_t)contents[6] & 0xFF) << 16) | 
                ( ((uint32_t)contents[5] & 0xFF) <<  8) | 
                ( ((uint32_t)contents[4] & 0xFF) <<  0) ;
    

    (The byte masking with 0xFF is necessary in case char is signed and negative, in which case the conversion to uint32_t will "sign extend" the value. TL;DR just stay clear of char as advised above.)

    Please note that this sorts data into the uint32_t according to some endian order (from 7 to 4, whatever that means in your case), which might be an issue, particularly if you drop the result in some data communication protocol. In theory, 8 bitters aren't using endianess, though in practice they do as soon as you have things like double accumulators or 16 bit index registers in the core. In such situations, PIC is Little Endian.

    Also note that %u is assuming unsigned int, a 16 bit type on your PIC. To print a uint32_t you should use "%"PRIu32 from inttypes.h, though %lu will work too.