wizard97 / Embedded_RingBuf_CPP

A simple C++ Ring (Circular) Buffer Queuing Library for Programming with Arduino's and other Embedded platforms
MIT License
101 stars 28 forks source link

Dev version is working on Arduino STM32 #14

Open GitMoDu opened 6 years ago

GitMoDu commented 6 years ago

I've tested with a Mapple Mini clone (using the ArduinoSTM32 platform), seems to work as fine as it does on ATmega328.

I'm pretty sure you can at least experimental compatibility to these micros and platform ( ARDUINO_ARCH_STM32 ).

wizard97 commented 6 years ago

Thanks for your testing. Were you using the dev branch, and did your testing make sure the interrupts masking worked properly?

Concurrency bugs are really subtle, and it could appear to be working fine, but not actually be masking interrupts properly.

GitMoDu commented 6 years ago

I'm not sure how I can properly test that. I can try modifying the the buffer while on a interrupt, while checking a global variable (maybe a parallel size counter?) on the main loop to see if any inconsistencies happen. Any suggestions?

wizard97 commented 6 years ago

So sorry, I just saw your response now!

All that really needs to be tested is that the ATOMIC() {} macro is working properly. What I would do is something like this:


bool in_critical = false;
ISR() {
if (in_critical) {
// This is bad, do something like turn on an LED
}

}

void loop() {
ATOMIC() {
in_critical = true;
delay(10);
in_critical = false;
}
delay(1);
}
LyricDev commented 2 years ago

Hi, I'm also hoping to use this library on ESP32. I've performed a test based on wizards suggestion above and it passed ok. I am not 100% sure it is a sufficient test as I am fairly new to the ESP32 architecture, having said that, I do believe I understand what it is doing and it does seem fine - the built-in LED remains unlit as expected. I include the code I used (via Arduino IDE) below. Test was done on a HiLetgo ESP-WROOM-32. I would be most curious to hear from wizard regarding whether the test is sufficient.


//#include <RingBufHelpers.h>
#include <RingBufCPP.h>

#define LED_PIN 2
//portMUX_TYPE timerMux = portMUX_INITIALIZER_UNLOCKED;
hw_timer_t* pTimer = timerBegin(0, 80, true); // Timer0, 1MHz scaled clock ticks (80MHz/80), gives uS timer resolution

volatile bool in_critical = false;
void IRAM_ATTR ISR() 
{
//  portENTER_CRITICAL_ISR(&timerMux);   
  if (in_critical) {
  // This is bad, do something like turn on an LED
  digitalWrite(LED_PIN, HIGH);
  }
//  portEXIT_CRITICAL_ISR(&timerMux);
}

void setup() {  
  pinMode(LED_PIN, OUTPUT);
  timerAttachInterrupt(pTimer, &ISR, true); // attach ISR 
  int duration = 10; // in us
  timerAlarmWrite(pTimer, duration, true); 
  timerAlarmEnable(pTimer);  
}

void loop() 
{  // test the buffer implementation against the esp32 architecture
  RB_ATOMIC_START
  in_critical = true;
  delay(10);
  in_critical = false;
  RB_ATOMIC_END
  delay(1);
}```
wizard97 commented 2 years ago

I don't think the normal master branch is safe on the ESP32, as the synchronization macros are NOPed. I do believe that the dev branch works properly with the ESP32, as it uses these macros for the synchronization. One caveat is that it only provides synchronization between threads/interrupts on the same core. If you are touching the same RingBuf instance from multiple cores, you will need to use a bigger hammer such as the portENTER_CRITICAL which under the hood is both a critical section (locking out interrupts/Tasks on the same core) and a spinlock (locking out the other core).

LyricDev commented 2 years ago

Thanks wizard. I'll try out the dev branch. I'm curious though, why my test above was seemingly successful. Is it a matter of pure luck? I left the code running for 15 mins or so to give it some time for races to occur...

Regarding the synchronisation macros, for my test above I simply enabled the ARDUINO_ARCH_ESP8266 flag, so I relied on that same implementation being sufficient on ESP32 - quite a leap! But it does seem to work as I say above, am I doing something stupid?

Also, I don't have a clear idea of which cores are allocated to which tasks, and finding out / doing something about it will be non-trivial for me. Therefore I intend to just use portENTER_CRITICAL in the MACRO definitions as this will probably suffice for my use case. Would appreciate your view on this approach.