What is wrong with this C code to control a LED using a sensor and a push button?

I am trying to program an ATmega328P to control an LED using an HC-SR04 sensor and a push button. My goal is for the sensor to turn ON the LED (like turn on a light in a room) when something passes within a 10 cm distance and to use the button to toggle the LED's state (turn it ON and OFF), but it doesn’t work.

The code seems correct to me, but when I test it, it doesn’t work as expected. The LED turns ON with the sensor, and when I press the button, it turns OFF, but after that, nothing else happens. Both the sensor and the button stop working, and I can’t understand why. Does anyone have any ideas on what might be causing this and what I need to change?

#include <avr/io.h>
#include <util/delay.h>

#define LED_PIN PD2
#define BUTTON_PIN PD3
#define TRIGGER_PIN PD5
#define ECHO_PIN PD6

void toggle_led() {
    PORTD ^= (1 << LED_PIN);
}

void turn_on_led() {
    if (!(PORTD & (1 << LED_PIN))) {
        PORTD |= (1 << LED_PIN);
    }
}

uint16_t measure_distance() {
    uint16_t duration = 0;
    PORTD |= (1 << TRIGGER_PIN);
    _delay_us(10);
    PORTD &= ~(1 << TRIGGER_PIN);

    while (!(PIND & (1 << ECHO_PIN)));
    while (PIND & (1 << ECHO_PIN)) {
        duration++;
        _delay_us(1);
    }

    uint16_t distance = (duration / 58);
    return distance;
}

int main(void) {
    DDRD |= (1 << LED_PIN);
    DDRD &= ~(1 << BUTTON_PIN);
    DDRD |= (1 << TRIGGER_PIN);
    DDRD &= ~(1 << ECHO_PIN);

    uint8_t button_pressed = 0;

    while (1) {
        if (!(PIND & (1 << BUTTON_PIN))) {
            if (!button_pressed) {
                toggle_led();
                button_pressed = 1;
            }
        } else {
            button_pressed = 0;
        }

        uint16_t distance = measure_distance();
        if (distance <= 10) {
            turn_on_led();
        }

        _delay_ms(100);
    }

    return 0;
}

Solution

Here is one potential problem:

Let's assume that measure_distance always returns a value <= 10. Either because the sensor correctly measured that distance or because of some problem with the sensor.

With measure_distance always returning a value <= 10, your main loop can be rewritten as:

while (1) {
    if (!(PIND & (1 << BUTTON_PIN))) {
        if (!button_pressed) {
            toggle_led();
            button_pressed = 1;
        }
    } else {
        button_pressed = 0;
    }

    turn_on_led();  // When measure_distance always returns <= 10

    _delay_ms(100);
}

So even when your button press turns the LED off, you will turn the LED back on instantly. Your eyes/brain won't notice any change - it's happening too fast.

You need to add a "big" delay after the button press. Perhaps start like:

            toggle_led();
            _delay_ms(1000);

I would also recommend that you test the sensor "stand-alone". Something like:

while (1) {
    uint16_t distance = measure_distance();
    if (distance <= 10) {
        toggle_led();     // NOTICE TOGGLE
        _delay_ms(1000);
    }
}

When an object is sufficiently close, the LED should toggle every second. Otherwise the LED should keep its state.