I just finished an easy SPI implementation that is used for 2 MCUs to communicate on a PCB. This communication is supposed to pass 16 bit Values from the master to a virtual register on the slave. The frame length is 64 bit.
The communication frame is build as followed:
bit0: read or write register. (0 == READ; 1 == WRITE)
bit1-bit15: register address
bit16-31: 0xFFFF to accomodate computation time on slave to provide data
bit32-47: register Value
bit 48-63: crc
The communication works flawlessly.
However, and this is what i do not understand, i am retrieving the address by taking the first 2 transmitted bytes and converting them to a uint16_t like this:
register_address = (uint16_t)(((byte0) & 0xFF) << 8 | ((byte1) & 0xFF));
This address is then used in a function that retrieves the value from a global register struct. As argument it should only accept values from the enum type "virtual_register_address_t"
typedef enum virtual_register_address_t
{
//READ/WRITE
REGISTER_ONE_ADD = 0,
REGISTER_TWO_ADD,
REGISTER_THREE_ADD,
//READ ONLY
ERROR_REGISTER_ADD
}virtual_register_address_t;
uint16_t get_virtual_register(virtual_register_address_t address)
{
uint16_t value = 0;
switch(address)
{
case(REGISTER_ONE_ADD):
value = virtual_register.registerOne;
break;
case(REGISTER_TWO_ADD):
value = virtual_register.registerTwo;
break;
case(REGISTER_THREE_ADD):
value = virtual_register.registerThree;
break;
case(ERROR_REGISTER_ADD):
value = virtual_register.errorRegister;
break;
default:
value = 0xFF;
break;
}
return value;
}
void set_virtual_register(virtual_register_address_t address, uint16_t data)
{
switch(address)
{
case(REGISTER_ONE_ADD):
virtual_register.registerOne = data;
break;
case(REGISTER_TWO_ADD):
virtual_register.registerTwo = data;
break;
case(REGISTER_THREE_ADD):
virtual_register.registerThree = data;
break;
case(ERROR_REGISTER_ADD):
break;
default:
break;
}
}
However, as some of you may already have recognized, i made a mistake by copying bit 0-15 from the spi frame instead of bit 1-15. So the address copied in a write case (first bit 1) should always be >=32768. The enum "virtual_register_address_t" is only defined up to 8. However, the code works flawlessly. It takes the parameter as "virtual_register_address_t" type even if the value is not regarded in the enum definition. It does not go to default state wihtin the switch even if the value is not regarded in the switch. It changes the values reliantly and sends them back just as recieved if i read the address after writing it.
I changed the getting of the register_address to
register_address = (uint16_t)(((byte0) & 0x7F) << 8 | ((byte1) & 0xFF));
and it still works.
To my understanding, the function "set_virtual_register" should do nothing if presented with values not in the switch case. But it reliantly set the values.
My question is, does this always happen if enums are taken as function parameter? How did it work when it shouldn't?
EDIT:
A user asked to add the function calls that use register address:
void spi_serialize(spi_handle_t* handle, virtual_register_address_t address, SPI_State read_or_write)
{
uint16_t crc = 0;
uint16_t data = 0;
switch(read_or_write)
{
case(READ):
data = get_virtual_register(address);
handle->dataTx[4] = (uint8_t)((data >> 8) & 0xff);
handle->dataTx[5] = (uint8_t)(data & 0xff);
break;
case(WRITE):
handle->dataTx[4] = (0xFF);
handle->dataTx[5] = (0xFF);
break;
default:
handle->dataTx[4] = (0xAA);
handle->dataTx[5] = (0xBB);
break;
}
//crc
crc = calculateCRC(handle->dataTxBase, SPI_FRAMESIZE-2);
handle->dataTx[SPI_FRAMESIZE-2] = ((crc >> 8) & 0XFF);
handle->dataTx[SPI_FRAMESIZE-1] = (crc & 0xFF);
}
void spi_deserialize(spi_handle_t* handle)
{
uint16_t register_address = 0;
uint16_t data = 0;
register_address = (uint16_t)(((handle->dataRx[0]) & 0xFF) << 8 | ((handle->dataRx[1]) & 0xFF));
data = (uint16_t)(((handle->dataRx[4]) & 0xFF) << 8 | ((handle->dataRx[5]) & 0xFF));
set_virtual_register(register_address, data);
}
CodePudding user response：
casedoes not need any parenthesises- The error is somewhere else
register_addressis not used anywhere in the code so it makes no difference what is there.- The
switch(...) casewill work exactly as you wrote it ie it will assign or get the value if theaddressis 0,1 or 2 (threecase has onlybreak)(
void spi_deserialize(spi_handle_t* handle)
{
uint16_t register_address = 0;
uint16_t data = 0;
register_address = (uint16_t)(((handle->dataRx[0]) & 0xFF) << 8 | ((handle->dataRx[1]) & 0xFF));
data = (uint16_t)(((handle->dataRx[4]) & 0xFF) << 8 | ((handle->dataRx[5]) & 0xFF));
set_virtual_register(register_address, data);
}
If the code above sets the registers it means that only two LS bits are set in those two bytes.
CodePudding user response：
Okay, i dived a little into the ARM Keil compiler.
An enum is either a char or an int. This depends on the range of values that are defined within. https://www.keil.com/support/man/docs/c51/c51_le_enum.htm https://www.keil.com/support/man/docs/c51/c51_ap_1bytescalar.htm
since i have way below 256 different definitions it is handled as a char. However, since the function parameter is an enum and not a uint8_t or char, calling the function with an uint16_t as parameter is not thrown as a warning or error at compile time. It accepts an uint16_t as parameter.
If a value greater than 255 is passed into the function it is truncated. Leaving only the lowest 8 bit. Therefore, it worked, even if the address passed to the function was not part of the enum. Because the lowest 8 bit were.
I tested the input and like described before, all values passed to the function are handled like
(parameter = value%6)
Thanks to all for taking the time. And sorry for not finding the answer before posting here.
If you do not think my answer is correct please elaborate. I would hate to have come to a wrong conclusion.