Are you supposed to be able to nest critical regions using CRITICAL_REGION_ENTER and CRITICAL_REGION_EXIT? Here I mean you have a function which uses a critical region to surround some work, and you call that function from within another critical region set up by another function. So you end up calling CRITICAL_REGION_ENTER twice, then CRITICAL_REGION_EXIT() twice.
Looking at the code for CRITICAL_REGION_ENTER, it makes sense. If the softdevice isn't enabled, it just turns interrupts off, else it relies on the softdevice to have turned off the application ones. However the code for CRITICAL_REGION_EXIT doesn't make so much sense. It's here..
#define CRITICAL_REGION_EXIT() \
if (CURRENT_INT_PRI != APP_IRQ_PRIORITY_HIGH) \
{ \
uint32_t ERR_CODE; \
__enable_irq(); \
ERR_CODE = sd_nvic_critical_region_exit(IS_NESTED_CRITICAL_REGION); \
if (ERR_CODE != NRF_ERROR_SOFTDEVICE_NOT_ENABLED) \
{ \
APP_ERROR_CHECK(ERR_CODE); \
} \
} \
}
This calls __enable_irq() directly, thus immediately turning on all interrupts so the critical region is effectively ended at this point even if you were nested and sd_nvic_critical_region_exit() was not going to disable the interrupts.
Should the code not do the inverse of the CRITICAL_REGION_ENTER() code and only call __enable_irq() in the case the softdevice isn't active? Something like
#define CRITICAL_REGION_EXIT()
if (CURRENT_INT_PRI != APP_IRQ_PRIORITY_HIGH)
{
uint32_t ERR_CODE= sd_nvic_critical_region_exit(IS_NESTED_CRITICAL_REGION);
if (ERR_CODE == NRF_ERROR_SOFTDEVICE_NOT_ENABLED)
{
__enable_irq();
}
else
{
APP_ERROR_CHECK(ERR_CODE);
}
}
}
Or do you have to ensure that you never call functions which end up nesting critical regions like this?