Repository: ClemensElflein/OpenMower
File: Firmware/LowLevel/src/main.cpp (C++)

Code before:
void setRaspiPower(bool power) {
    // Update status bits in the status message
    status_message.status_bitmask = (status_message.status_bitmask & 0b11111101) | ((power & 0b1) << 1);
    digitalWrite(PIN_RASPI_POWER, power);
}

void updateEmergency() {
    if (millis() - last_heartbeat_millis > HEARTBEAT_MILLIS) {
        emergency_latch = true;
        ROS_running = false;
    }
    uint8_t last_emergency = status_message.emergency_bitmask & LL_EMERGENCY_BIT_LATCH;
    uint8_t emergency_state = 0;

    // Check all "stop" mode emergency inputs (halls)
    bool stop_pressed = false;
    size_t i;
    for (i = 0; i < MAX_HALL_INPUTS; i++) {
        if (stop_halls[i] == 0xff) break;                                                            // End marker reached
        stop_pressed = halls[stop_halls[i]]() ^ llhl_config.hall_configs[stop_halls[i]].active_low;  // Get hall value and apply active_low level (invert)
        if (stop_pressed) break;                                                                     // Break at first detected stop
    }

    // Handle emergency "Stop" buttons
    if (stop_pressed) {
        if (button_emergency_started == 0) button_emergency_started = millis();  // If we just pressed, store the timestamp
    } else {
        button_emergency_started = 0;  // Not pressed, reset the time
    }
    if (button_emergency_started > 0 && (millis() - button_emergency_started) >= BUTTON_EMERGENCY_MILLIS) {
        emergency_state |= LL_EMERGENCY_BIT_STOP;
    }

    // Check all "lifted" mode emergency inputs (halls)
    int num_lifted = 0;
    for (i = 0; i < MAX_HALL_INPUTS; i++) {
        if (lift_halls[i] == 0xff) break;                                                 // End marker reached
        if (halls[lift_halls[i]]() ^ llhl_config.hall_configs[lift_halls[i]].active_low)  // Get hall value and apply active_low level (invert)
            num_lifted++;
        if (num_lifted >= 2) break;  // Break once two wheels are lifted
    }

    // Handle lifted (>=2 wheels are lifted)
    if (num_lifted >= 2) {
        if (lift_emergency_started == 0) lift_emergency_started = millis();  // If we just lifted, store the timestamp
    } else {
        lift_emergency_started = 0;  // Not lifted, reset the time
    }

    // Handle tilted (at least one wheel is lifted)
    if (num_lifted >= 1) {

Review comment (suggestion):
I personally prefer the comment on its own line, especially if it explains the branch (i.e. the situation which brought us here) more than the particular line. What I mean is that it's obvious that "button_emergency_started = millis()" stores the timestamp, we wouldn't need any comment for that, but the "just pressed" comment *is* helpful because it explains when "button_emergency_started == 0" can happen.

Code after:
    status_message.status_bitmask = (status_message.status_bitmask & 0b11111101) | ((power & 0b1) << 1);
    digitalWrite(PIN_RASPI_POWER, power);
}

void updateEmergency() {
    if (millis() - last_heartbeat_millis > HEARTBEAT_MILLIS) {
        emergency_latch = true;
        ROS_running = false;
    }
    uint8_t last_emergency = status_message.emergency_bitmask & LL_EMERGENCY_BIT_LATCH;
    uint8_t emergency_state = 0;

    // Check all emergency inputs (halls)
    bool stop_pressed = false;
    int num_lifted = 0;
    for (const auto &hall : halls) {
        if (hall.get_value() ^ hall.config.active_low) {  // Get emergency input value and invert if low-active
            if (hall.config.mode == HallMode::STOP) {
                stop_pressed = true;
            } else if (hall.config.mode == HallMode::LIFT_TILT) {
                num_lifted++;
            }
            // Unlikely to happen, but there's no more emergency than stop and multiple-wheels lifted
            if (stop_pressed && num_lifted >= 2)
                break;
        }
    }

    // Handle emergency "Stop" buttons
    if (stop_pressed) {
        if (button_emergency_started == 0) {
            button_emergency_started = millis();  // Just pressed, store the timestamp for debouncing
        } else if (button_emergency_started > 0 && (millis() - button_emergency_started) >= BUTTON_EMERGENCY_MILLIS) {
            emergency_state |= LL_EMERGENCY_BIT_STOP;  // Debounced
        }
    } else {
        button_emergency_started = 0;  // Not pressed, reset the time
    }

    // Handle lifted (>=2 wheels are lifted)
    if (num_lifted >= 2) {
        if (lift_emergency_started == 0) lift_emergency_started = millis();  // If we just lifted, store the timestamp
    } else {
        lift_emergency_started = 0;  // Not lifted, reset the time
    }

    // Handle tilted (at least one wheel is lifted)
    if (num_lifted >= 1) {
        if (tilt_emergency_started == 0) tilt_emergency_started = millis();  // If we just tilted, store the timestamp
    } else {
        tilt_emergency_started = 0;  // Not tilted, reset the time