DIY Powermeter nRF52840/AD7799

[captainfips]

I am developing a bicycle powermeter using the XIAO BLE nRF52840 Sense Breakout board with an AD7799 on a dedicated PCB i also made. I use Segger Embedded Studio v5.42a and nRF5_SDK_17.1.0. For ANT+ communication I use the S212 7.0.1 Softdevice. BPWR Sensor example compiles and runs without any problems.

The ADC is controlled via SPIM and uses the Miso line as a Data Ready Indicator by pulling it low. I want to use that functionality with one Gpiote Input HITOLO Interrupt on the Miso line, and when the Interrupt occurs, I deactivate the gpiote_in_event, to not trigger the interrupt again during SPIM communication. When the 3 bytes have been received, the spim event handler reenables the gpiote in event when beeing called after the transmission and sums puts the three bytes received back together into a 32 bit Integer and increments a counter variable as well.

I use the ant_bpwr_evt_handler to call a function in my ad7799 library which uses the accumulated data read from the adc and the counter value that were already received since the last bpwr page update event. My problem is, what happens if an bpwr page update event occurs, after I wrote the new SPI data to the 32 bit Integer but before the counter is incremented? Then the calculated average of the samples in that time period would be incorrect... Is there some simple way of not taking the values when they are currently being written to? I cannot use a while loop with a boolean I set while changing the values, because the bpwr event handler interrupt has a higher priority than the spi event handler, so that would end up in an endless loop. An if statement would probably work, but I'd miss a whole bpwr page update cycle...

I would appreciate other ideas or solutions as well. Thanks in advance.

[Einar Thorsrud]

Hi,

Is there some simple way of not taking the values when they are currently being written to?

I did not get a full understanding of your logic here, but it seems like you could benefit from a mutex here, to protect access to the data so that they are only accessed from a single place at a single time. See <nRF5 SDK>\components\libraries\mutex\nrf_mtx.h for the API, and you can search for "nrf_mtx_trylock" to see a couple of examples.

[captainfips]

Hi,

yesterday I tried the solution with a boolean variable that is part of the struct, of which the data always has to be accessed at once.
There are two cumulative sums and one counter in the struct and if the reading access in the Softdevice Interrupt happens while the spim interrupt began to write to the struct members it could happen that the sums have already been changed, but the counter hasn't. That would result in a wrong average value. I now added a bool "locked", and always set that to true when I start writing to or reading from the other struct members. After the access is finished I reset locked to false to enable accessing it again. 

if (!adc_values.locked)
{
  adc_values.locked = true;
  adc_values.max_value = fmaxf(adc_values.max_value, f_data);
  adc_values.positive_sum += fmaxf(0, f_data); //Maybe faster than a branch?
  adc_values.negative_sum += fminf(0, f_data);
  adc_values.num_values++;
  adc_values.locked = false;
}

I think that should work for now, but it would be possible, that I miss either one reading access or one writing access, if the new access happens, while the struct is currently being accessed.

For what I found of the nrf_mtx library, I think the functionality is more or less the same?

I also saw the app_util_platform.h file, which defines CRITICAL_REGION_ENTER() and CRITICAL_REGION_EXIT(). What do these functions exactly do?

Thanks in advance.

[Einar Thorsrud]

Hi,

captainfips said:

I think that should work for now, but it would be possible, that I miss either one reading access or one writing access, if the new access happens, while the struct is currently being accessed.

If that is a problem, you could consider adding some special handling with a temporary buffer that could be used in the mean time if the added complexity is worth it.

captainfips said:

For what I found of the nrf_mtx library, I think the functionality is more or less the same?

More or less, but not exactly. The problem with your implementation is that you read locked first, then write to it later. So with bad luck, there is an interrupt between reading (and seeing that it is not locked) and writing that it is locked, and in that case the mechanism breaks down. That is why nrf_mtx is better, as it uses nrf_atomic_u32_fetch_store() which guarantees that reading and writing happens uninterrupted). (You could also just use a critical section if you like, though, which is one of the implementations used there).

captainfips said:

CRITICAL_REGION_ENTER() and CRITICAL_REGION_EXIT(). What do these functions exactly do?

In practice the implementation just globally disables interrupts (when entering a critical section) and re-enables interrupts when exiting a critical section. That way, you are guaranteed that the code within the critical section is never interrupted.

[captainfips]

Thank you, that is a good explanation. I think for now, I will change my code to use a mutex, to keep it simpler.

In my implementation, would it even matter if the interrupt occurs between reading and then setting the bool? Because even if the first access would have tried to lock the struct, the second access would be the same, wouldn't it? Because in the beginning of the second access, none of the critical struct members have changed yet.
Furthermore, if the second interrupt happens directly after the boolean was set, it would just leve the struct as it is and return. So I don't think it would even matter, if the access occured after first boolean check.

[Einar Thorsrud]

It is generally not easy to see if it would be problematic or not. How does the other code that access the same buffer look like? And what if the caller code would change somehow (for instance going from one interrupt priority to another, or perhaps if you had a preemptive RTOS, etc). So I would recommend doing this properly the first time as it does not take more time or cost much in term of CPU time, but it could save you trouble and difficult to find bugs in the future (not necessarily here, who knows, but getting in to the habit of always doing such things in a safe manner is a good idea).

[captainfips]

I totally agree with you, it definitely is more safe with a mutex instead of the boolean. Here is the other accessing part of the code:

if (!adc_values.locked)
{
  adc_values.locked = true;
  float adc_sum = adc_values.positive_sum + adc_values.negative_sum;
  float mV_avg = adc_sum / fmaxf((float)adc_values.num_values, 1.f);
  float weight_avg = fmaxf(mV_avg * adc_kg_per_mV, 0);
  float torque_avg = (weight_avg)*1.71675; // 9.81 * 0.175 for Nm
  torqueEff[0] = torqueEff[1] = round((200.f * (adc_sum)) / adc_values.positive_sum);
  pedalSmoothness[0] = pedalSmoothness[1] = round(200.0 * adc_sum / adc_values.max_value);
  *instPWR = (uint16_t)round(torque_avg * TWO_PI * lastRPS * 2); // calc instant pwr
  *accPWR += *instPWR;
  *instCAD = (uint8_t)round(60 * lastRPS); // Calc cadence of last rotation in U/min
  *evt_counter++;
  adc_values.locked = false;
  return true;
}
else return false;