madhephaestus / ESP32Servo

Arduino-compatible servo library for the ESP32
133 stars 53 forks source link

Using double instead of floats, as ESP32 doesn't allows floats in ISR #24

Closed mathix420 closed 2 years ago

mathix420 commented 2 years ago

These changes shouldn't make any difference in functioning, but I would recommend testing it in other use-cases. Hope these changes can help others!

More context: https://github.com/espressif/arduino-esp32/issues/3661#issuecomment-578224260

Have a nice day!

madhephaestus commented 2 years ago

double will actually make the problem worse, not better... I think this all would need to be changed over to fixed point or integer math. Ill need to look into the issue. Do you have an .ino project to demonstrate a problem?

Also, in terms of design pattern, on ESP32 its better to use threads than interrupts directly.

mathix420 commented 2 years ago

double will actually make the problem worse, not better...

For my usecase, this change has fixed the issue. As I understood double in computed during code compilation, while floats are computed during runtime. So it probably tried to use same memory space or same cpu core in parallel. That's how I understood my issue, I'm sure you're way more qualified than me on the subject. Just put some more details so you can understand why I've done that.


Do you have an .ino project to demonstrate a problem?

Sure here is an example project I'm working on. (below)


on ESP32 its better to use threads than interrupts directly

Oh great, didn't know we could use threads on such a device.

#include <ArduinoWebsockets.h>
#include <ESP32Servo.h>
#include <WiFi.h>

// States constants
#define CLOSE_STATE     0
#define CLOSING_STATE   1
#define OPENNING_STATE  2
#define OPEN_STATE      3

// Servo constants
#define SERVO_ZERO_POS  55
#define SERVO_SPEED     40
#define SERVO_PIN       15

// Pin constants
#define TOP_SWITCH      12
#define BOTTOM_SWITCH   13
#define LED_BUILTIN     4

const char *ssid =      "YOUR_SSID";
const char *password =  "YOUR_PSWD";
volatile int DOOR_STATE = CLOSE_STATE;
volatile bool NOTIFIED = true;

// Init WS
using namespace websockets;
WebsocketsServer server;
WebsocketsClient client;

// Init servo
Servo myservo;

void    setup()
{
    // Setup serial
    Serial.begin(115200);

    // Setup servo
    ESP32PWM::allocateTimer(0);
    ESP32PWM::allocateTimer(1);
    ESP32PWM::allocateTimer(2);
    ESP32PWM::allocateTimer(3);
    myservo.setPeriodHertz(50);
    myservo.attach(SERVO_PIN, 1000, 2000);

    // Setup LED
    pinMode(LED_BUILTIN, OUTPUT);

    // Setup interrupts
    pinMode(TOP_SWITCH, INPUT_PULLUP);
    pinMode(BOTTOM_SWITCH, INPUT_PULLUP);
    attachInterrupt(TOP_SWITCH, openned_event, FALLING);
    attachInterrupt(BOTTOM_SWITCH, closed_event, FALLING);

    // Start LED for debugging
    digitalWrite(LED_BUILTIN, HIGH);

    // Connect to wifi
    WiFi.begin(ssid, password);

    // Wait some time to connect to wifi
    for (int i = 0; i < 15 && WiFi.status() != WL_CONNECTED; i++)
    {
        Serial.print(".");
        delay(1000);
    }

    // Debug
    Serial.println("");
    Serial.println("WiFi connected");
    Serial.println("IP address: ");
    Serial.println(WiFi.localIP());

    server.listen(80);
    Serial.print("Is server live? ");
    Serial.println(server.available());

    // All done so LED is off now
    digitalWrite(LED_BUILTIN, LOW);
}

void    loop()
{
    if (!client.available())
        client = server.accept();
    else if (DOOR_STATE == OPEN_STATE || DOOR_STATE == CLOSE_STATE)
    {
        if (!NOTIFIED) notify_client();
        WebsocketsMessage msg = client.readBlocking();
        Serial.print("Got Message: ");
        Serial.println(msg.data());

        if (msg.data() == "OPEN_DOOR")
            client.send(open_door());
        else if (msg.data() == "CLOSE_DOOR")
            client.send(close_door());
        else if (msg.data())
            client.send("NOT_UNDERSTOOD");
    }
    delay(1000);
}

void    notify_client()
{
    if (DOOR_STATE == OPEN_STATE)
        client.send("DOOR_OPENED");
    else if (DOOR_STATE == CLOSE_STATE)
        client.send("DOOR_CLOSED");
    NOTIFIED = true;
}

char    *close_door()
{
    if (DOOR_STATE != OPEN_STATE)
        return "STATE_ERROR";
    DOOR_STATE = CLOSING_STATE;
    myservo.write(SERVO_ZERO_POS + SERVO_SPEED);
    return "CLOSING_DOOR";
}

char    *open_door()
{
    if (DOOR_STATE != CLOSE_STATE)
        return "STATE_ERROR";
    DOOR_STATE = OPENNING_STATE;
    myservo.write(SERVO_ZERO_POS - SERVO_SPEED);
    return "OPENNING_DOOR";
}

void    openned_event()
{
    if (DOOR_STATE != OPENNING_STATE && DOOR_STATE != CLOSING_STATE)
        return;
    myservo.write(SERVO_ZERO_POS);
    NOTIFIED = false;
    DOOR_STATE = OPEN_STATE;
    Serial.println("DOOR_OPENNED");
}

void    closed_event()
{
    if (DOOR_STATE != CLOSING_STATE)
        return;
    myservo.write(SERVO_ZERO_POS);
    NOTIFIED = false;
    DOOR_STATE = CLOSE_STATE;
    Serial.println("DOOR_CLOSE");
}

Dependancy:

With this code and the unmodified lib, I got **Guru Meditation Error: Core 1 panic'ed (Coprocessor exception)**. With doubles instead of floats, this work fine.

madhephaestus commented 2 years ago

It turns out you are right, double doesnt fire up the fpu and so doesnt produce the fpu in isr bug! Its a small performance hit to trade off for being able to manipulate the seros and PWM in ISR's.