Uno ICR/Timer0/ISR issues

hello all. I am pretty new to Arduino but I am at least familiar with c++. I found some code online for using an atmega168 to store the messages sent by my car's stock headunit (uses 5v single line PWM). I am trying to modify it to work with an Arduino Uno (R3), which I feel partially successful at.

The PWM is as follows: logical 1 is line pulled down to 0v for ~1.7ms (then a wait) and logical 0 is line pulled down to 0v for ~.4 ms (then a wait). The idea is that an interrupt is triggered every 4.6ms. which then sends the collected byte(s) to a computer via serial (using the iDE's serial monitor to view). I could really use some help completing the arduino conversion. the PWM line is connected to d8 on the Uno.

#include "Arduino.h"
#include <avr/io.h>
#include <avr/interrupt.h>


#define buffer_size 16
#define intPin 8
volatile uint8_t buffer[buffer_size];
volatile uint8_t buffer_pos = 0;
volatile uint8_t byte_pos = 0;
volatile uint8_t pin = 0;
   

void send_byte( uint8_t byte )
{
    loop_until_bit_is_set( UCSR0A, UDRE0 ); // Wait for buffer to be empty
    UDR0 = byte;                            // Load data into the buffer
}

void setup()
{
    // Set up serial I/O
	Serial.begin(19200);
    // 9600 baud is too slow to transmit some of the larger commands in the
    // ~10ms window we're given. At 19200 baud we don't run into any problems.
        InitializeIO();
}

void loop() {
   // Do nothing, everything is handled by interrupts
}

void InitializeIO() {
    cli();          // disable global interrupts
    TCCR0A = 0;     // set entire TCCR0A register to 0
    TCCR0B = 0;     // same for TCCR0B
    // 8 bit timer (prescaler = 256)
    TCCR0A |= (1 << WGM02);
    TCCR0B |= (1 << CS02);
    OCR0A = 288; // Trigger an interrupt every ~4.6ms
    TIMSK0 |= (1 << OCIE0A);
    // Interrupt on pin change
    PCICR |= (1 << PCIE0);
    PCMSK0 = (1 << PCINT0); // We want pin 8 (PB0)
    // Enable interrupts
    sei();    
}

// Pin change interrupt
ISR(PCINT0_vect)
{
    uint8_t elapsed_time = TCNT0;  // Store the timer value
    digitalWrite (pin, digitalRead(intPin)); // Store the pin value
    
    // If the bus is pulled to ground, a bit is starting to be transmitted
    // so the timer is reset and we can return while we wait for the 
    // bus to be pulled back to 5V.
    if (!pin){
        TCNT0 = 0;
        return;
    }
    
    // A logical 1 is when the bus is held low for ~1.7ms so we accept
    // anything from ~1.4ms to ~2ms as a logical 1.
    if (elapsed_time > 87 && elapsed_time < 125)
        buffer[buffer_pos] |= 1<<(7-byte_pos);
    
    // Anything else is assumed to be a 0, increase the byte position
    byte_pos++;
    
    // We've filled up a byte, move to the next one
    if (byte_pos > 7){
        byte_pos = 0;
        buffer_pos++;
        buffer[buffer_pos] = 0; // very important, clear the old value
    }
}

// Timer compare match interrupt
ISR(TIMER0_COMPA_vect)
{
    if (!buffer_pos){
        // clean up any bits that have been stored because of noise
        // (very useful on startup)
        byte_pos = 0;
        buffer[0] = 0;
        return;
    }
    
    if (byte_pos) // sometimes a byte is only half-filled, make sure to send it
        buffer_pos++;
    
    for (uint8_t i=0; i<buffer_pos; i++)
        send_byte(buffer[i]);
    
    // Send the footer
   Serial.write(0xff);
   Serial.write(buffer_pos*8 + byte_pos);
    
    buffer_pos = 0;
    byte_pos = 0;
    buffer[0] = 0;
}

the original code is attached as well.

dump.txt (2.69 KB)

    digitalWrite (pin, digitalRead(intPin)); // Store the pin value

That is NOT what that code does!

   Serial.write(0xff);
   Serial.write(buffer_pos*8 + byte_pos);

Serial IO requires interrupts to be enabled. That is not the case in an ISR.

Judging by your description I would rather use a CHANGE interrupt and note the elapsed time. From the start of the sequence (and in particular you when you get a high-to-low change), note the current time by calling micros(). Then when the next interrupt occurs (presuming it is now low to high) call micros() again. Allowing for a fudge-factor, you could say that something like 200 to 800 µS would be a zero, and an interval of 800 to 2000 µS would be a one. Just collect those, until you have enough bits.

And as PaulS says, don't attempt to print anything in the ISR. Just set a "done" flag when appropriate.