MT6835.cpp: SPI sensor write actions fail on STM32 due to non-portable union usage

[c031917]

When using the ABZ outputs you have to program the resolution (factory setting is 1PPR) via SPI. Also persisting the register settings in EEPROM needs write actions. Setting the Autocal. speed range for SPI or ABZ mode is also affected (Options4). I found out that Angle Read works on STM32, but all write operations failed. By tracking the SPI transfers I noticed that the 3 bytes per command do not arrive in the right sequence at the sensor. The reason for this is the union in MT6835.h: union MT6835Command { struct { uint32_t cmd:4; uint32_t addr:12; uint32_t data:8; uint32_t unused:8; }; uint32_t val; };

This code will work on 8bit MCUs, but not on most 32bit MCUs, as there is no warrranty in which sequence the bit fields are stored in memory.
When filling the command buffer directly with the right command bytes, my sensor works: bool MT6835::writeEEPROM(){ delay(1); // wait at least 1ms

MT6835Command cmd;

/ cmd.cmd = MT6835_OP_PROG; cmd.addr = 0x000; transfer24(&cmd); return cmd.data == MT6835_WRITE_ACK; / //mx fix nonportable union uint32_t my_cmd2 = 0x0000C0; // command to write EEPROM mytransfer24(&my_cmd2); return cmd.data == MT6835_WRITE_ACK;

The byte which is clocked out first must be on the right - here 0xC0.

Similar quick fix here:

void MT6835::setABZResolution(uint16_t res){ /* uint8_t hi = (res >> 2); MT6835ABZRes lo = { .reg = readRegister(MT6835_REG_ABZ_RES2) }; lo.abz_res_low = res & 0x3F;

writeRegister(MT6835_REG_ABZ_RES1, hi);
writeRegister(MT6835_REG_ABZ_RES2, lo.reg);

*/ //mx fix nonportable union uint32_t my_cmd = 0xFF0760; // set ABZ resolution to 16384 mytransfer24(&my_cmd); my_cmd = 0xFC0860; mytransfer24(&my_cmd); };

and here: void MT6835::setOptions4(MT6835Options4 opts){ // MT6835Options4 val = getOptions4(); // val.gpio_ds = opts.gpio_ds; // val.autocal_freq = opts.autocal_freq; // writeRegister(MT6835_REG_OPTS4, val.reg); //mx fix nonportable union, ignoring val.gpio_ds uint32_t my_cmd = 0x600e60; // set autocal freq to 0x6 = 50-100RPM mytransfer24(&my_cmd); };

void MT6835::mytransfer24(uint32_t* outValue) { if (nCS >= 0) digitalWrite(nCS, LOW); spi->beginTransaction(settings); spi->transfer(outValue, 3); spi->endTransaction(); if (nCS >= 0) digitalWrite(nCS, HIGH); };

The left the other write commands, just wanted to make ABZ 14bit work.

So there should be implemented a different data structure which is also portable to all MCUs.

Regards Husky

[runger1101001]

Thanks a lot for testing out the driver and reporting this!

are you sure it is a MCU specific difference? Because actually both AVR and STM32 should both be little-endian? Did you compare it to AVR 8bit? I have a fear it is also not working there because the sensor expects big-endian… ESP32 and nRF52 are also little endian, so actually all our architectures are little endian, which is why I went with the bit-field approach…

[c031917]

I did not yet test this on AVR. I was just assuming that it works there ;-) Need some time to set up something - vacancy.. Could it be that the uint_32 type is mapped differently on the MCUs? High word first or last?

[runger1101001]

Yes, I think so. I'm on vacation so I can't test anything at the moment, but I was looking at the code for the SPI - the SPI.transfer16() method checks the configured byte-order (MSB, LSB) and does a byte-swap if needed. The SPI.transfer() function I used for the 24bit transfers of the MT6835 doesn't do any byte-swapping, so I guess I have to add this in myself.

That's a hypothesis, but I'll test it out when I get back next week...

[runger1101001]

So I have it working for me now, tested on STM32 - so at least on ARM MCUs it should be ok :-)

The problem was twofold:

1. The bitfields were all defined in the wrong order, I reversed them
2. The transfer24 function didn't take into account the little-endian byte order, now it swaps the bytes sent and received

Now the bitfields seem to work.

I agree that it probably is not the best way to do it, but in the end I find the representation both easier to understand and debug, and ultimately also less error-prone than directly working on the bits with masks and shifting. It's apparently an ongoing discussion now for many years, that even the original authors of the C language have been involved in. Should bitfields be used for things like memory-mapped registers, peripheral drivers, etc? I guess I'm in the camp of yes, they should :-)

Anyways, since all our Arduino platforms are apparently little-endian and compiled by gcc this solution should be portable enough.

[runger1101001]

Fixed in 9b03b5d9e9a298714a90b18f2842c639c21bb693

[runger1101001]

This has been merged and released