Code_review: fixed minor issues and added github action

.github/workflows/build.yml

@@ -0,0 +1,28 @@
+name: Build
+
+on: [push, pull_request]
+
+jobs:
+  build:
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: actions/setup-python@v5
+        with:
+          python-version: '3.11'
+
+      - name: Cache PlatformIO
+        uses: actions/cache@v4
+        with:
+          path: ~/.platformio
+          key: ${{ runner.os }}-pio-${{ hashFiles('platformio.ini') }}
+          restore-keys: |
+            ${{ runner.os }}-pio-
+
+      - name: Install PlatformIO
+        run: pip install --upgrade platformio
+
+      - name: Build firmware
+        run: pio run

.gitignore

@@ -1,6 +1,12 @@
 # Created by https://www.toptal.com/developers/gitignore/api/visualstudiocode,c++,platformio
 # Edit at https://www.toptal.com/developers/gitignore?templates=visualstudiocode,c++,platformio
 
+### macOS ###
+# General
+.DS_Store
+.AppleDouble
+.LSOverride
+
 ### C++ ###
 # Prerequisites
 *.d
@@ -46,7 +52,6 @@
 .vscode/*
 !.vscode/settings.json
 !.vscode/tasks.json
-!.vscode/launch.json
 !.vscode/extensions.json
 !.vscode/*.code-snippets
 

README.md

@@ -35,13 +35,16 @@ This effectively converts HP RedEye IR communication into a standard UART data s
 
 ### Connections (Typical)
 
-- TSMP58000 OUT → RP2040 GPIO (configurable)
-- TSMP58000 VCC → 3.3V
-- TSMP58000 GND → GND
-- RP2040 UART0 TX → External UART device RX
-- RP2040 GND → Common ground
+| Signal | Pin | Notes |
+|---|---|---|
+| TSMP58000 OUT | GPIO 2 (`DATA_PIN`) | IR signal input |
+| TSMP58000 VCC | 3.3V | |
+| TSMP58000 GND | GND | |
+| UART0 TX | GPIO 0 | 115200 baud, connect to RX of target device |
+| GND | GND | Common ground with target device |
+| GPIO 3 (`HALF_BIT_RECEIVED_PIN`) | optional | Debug probe — toggles on each half-bit, useful with a logic analyzer |
 
-Pin assignments can be changed in firmware if needed.
+Pin assignments can be changed by editing the `#define` constants at the top of `src/main.cpp`.
 
 ---
 
@@ -67,8 +70,27 @@ The UART output contains decoded RedEye payload bytes in real time.
 
 ## Examples
 
-Print LCD pixel-by-pixel (char 27 is DRAW mode pointer): ![Image alt](https://github.com/Fyfar/hp_redeye_rp2040/raw/main/prlcd.png)
-Print RPN stack and single value as value: ![Image alt](https://github.com/Fyfar/hp_redeye_rp2040/raw/main/print%20stack.png)
+Print LCD pixel-by-pixel (byte 27 triggers drawing mode):
+![HP 28C LCD printed pixel-by-pixel via RedEye drawing mode](https://github.com/Fyfar/HP_RedEye/raw/main/prlcd.png)
+
+Print RPN stack and a single numeric value:
+![HP 28C RPN stack printed as text via RedEye](https://github.com/Fyfar/HP_RedEye/raw/main/print%20stack.png)
+
+---
+
+## Building & Flashing
+
+Requires [PlatformIO](https://platformio.org) (e.g. via the VS Code extension or CLI).
+
+```bash
+# Build and upload
+pio run --target upload
+
+# Open serial monitor (115200 baud)
+pio device monitor
+```
+
+The USB serial port (`Serial`) outputs debug info. `Serial1` (UART0, GPIO 0) outputs the decoded bytes.
 
 ---
 

src/main.cpp

@@ -4,17 +4,19 @@
 #define MAX_HALF_BITS 30
 #define PWM_IRQ_PIN 15  // any existing pin
 
+// HP RedEye error correction parity masks — each covers a subset of the 8 data bits;
+// a transmitted EC bit is set if the count of 1s under that mask is odd.
 #define EC_MASK1 0b01111000
 #define EC_MASK2 0b11100110
 #define EC_MASK3 0b11010101
 #define EC_MASK4 0b10001011
 
-#define DRAWING_MODE_BYTE 27
+#define DRAWING_MODE_BYTE 27             // ASCII ESC (0x1B)
 #define DRAWING_BLOCKS 4                 // LCD is 32 pixels height, and each block is 8 pixels (1 byte), so we need 4 blocks to represent the full height of the LCD
 #define MAX_DRAWING_BYTES_PER_BLOCK 166  // represents max line width of HP 82240A/B printer
 
-#define END_OF_TRANSMISSION_BYTE 4
-#define ASCII_OFFSET 128
+#define END_OF_TRANSMISSION_BYTE 4  // ASCII EOT (0x04)
+#define ASCII_OFFSET 128            // HP Roman-8 extended characters start at byte value 128
 
 #include <Arduino.h>
 #include <hardware/pwm.h>
@@ -105,9 +107,9 @@ void __no_inline_not_in_flash_func(dataISR)(uint gpio, uint32_t events) {
 }
 
 uint_fast8_t checkBit(const bool firstHalfBit, const bool secondHalfBit) {
-    if (firstHalfBit == 0 && secondHalfBit == 1) {
+    if (!firstHalfBit && secondHalfBit) {
         return 0;  // Logical '0'
-    } else if (firstHalfBit == 1 && secondHalfBit == 0) {
+    } else if (firstHalfBit && !secondHalfBit) {
         return 1;  // Logical '1'
     } else {
         return 255;  // Invalid bit (error)
@@ -116,7 +118,6 @@ uint_fast8_t checkBit(const bool firstHalfBit, const bool secondHalfBit) {
 
 bool validateStartHalfBits(const bool* buffer) {
     for (uint8_t i = protocolConfig.startHalfBitsOffset; i < protocolConfig.startHalfBits; i++) {
-        uint8_t bitPosition = protocolConfig.startHalfBits - 1 - i;
         if (!buffer[i]) return false;  // Start half-bits should be all 1s, if not, it's an error
     }
 
@@ -142,8 +143,8 @@ uint_fast8_t extractErrorCorrectionBits(const bool* buffer) {
             return 255;  // Return 255 to indicate an error in error correction bits
         }
 
-        uint8_t bitPosition = (i - 3) / 2;                       // Calculate bit position for error correction
-        errorCorrectionBits |= (bitValue << (3 - bitPosition));  // Shift and combine bits into errorCorrection variable
+        uint8_t bitPosition = (i - protocolConfig.errorCheckingBitsOffset) / 2;
+        errorCorrectionBits |= (bitValue << (3 - bitPosition));
     }
 
     Serial.print("Error correction value: 0b");
@@ -173,27 +174,13 @@ uint_fast8_t extractDataByte(const bool* buffer) {
 
         if (bitValue == 255) return 255;  // Return 255 to indicate an error in data bits
 
-        uint8_t bitPosition = (i - 11) / 2;           // Calculate bit position for data bits
-        dataByte |= (bitValue << (7 - bitPosition));  // Shift and combine bits into a byte
+        uint8_t bitPosition = (i - protocolConfig.dataBitsOffset) / 2;
+        dataByte |= (bitValue << (7 - bitPosition));
     }
 
     Serial.print("Decoded byte: 0b");
     Serial.println(dataByte, BIN);
 
-    if (drawMode) {
-        return dataByte;  // If we are in drawing mode, we will return the raw byte value for processing as drawing data, without interpreting it as ASCII or Roman symbols, since the drawing data can contain any byte value and is not meant to be interpreted as text.
-    }
-
-    if (dataByte >= ASCII_OFFSET + 18) {
-        Serial.println("Decoded byte is out of bounds for defined ROMAN_8_SYMBOLS.");
-    } else if (dataByte >= ASCII_OFFSET && dataByte < ASCII_OFFSET + 18) {
-        Serial.print("Decoded byte (roman symbol): ");
-        Serial.println(ROMAN_8_SYMBOLS[dataByte - ASCII_OFFSET]);
-    } else {
-        Serial.print("Decoded byte (ascii): ");
-        Serial.println((char)dataByte);
-    }
-
     return dataByte;
 }
 
@@ -209,8 +196,8 @@ void sendSymbolDataByte(uint_fast8_t dataByte) {
 
 void disableDrawingMode() {
     if (drawMode) {
-        Serial.println("Drawing mode OFF");  // Send drawing mode status over Serial (USB)
-        Serial1.println(DRAWING_MODE_BYTE);  // Send drawing mode status over Serial1 (
+        Serial.println("Drawing mode OFF");
+        Serial1.println(DRAWING_MODE_BYTE);
     }
     drawMode = false;
 }
@@ -224,16 +211,16 @@ void enableDrawingMode() {
 }
 
 void sendDrawBufferToUART() {
-    uint8_t buffer[bytesToDrawInCurrentBlock + 1];  // +1 for null terminator
+    uint8_t buffer[MAX_DRAWING_BYTES_PER_BLOCK + 1];  // +1 for trailing newline
 
-    Serial.println("Sending drawing buffer to UART:");  // Log the action of sending the drawing buffer
+    Serial.println("Sending drawing buffer to UART:");
     for (uint8_t bitIndex = 0; bitIndex < 8; bitIndex++) {
         for (uint8_t index = 0; index < bytesToDrawInCurrentBlock; index++) {
             uint8_t mask = 1 << bitIndex;
             buffer[index] = (drawingBuffer[index] & mask) ? '.' : ' ';
         }
-        buffer[bytesToDrawInCurrentBlock] = '\n';                           // Null-terminate the buffer to safely print as a string
-        uart_write_blocking(uart0, buffer, bytesToDrawInCurrentBlock + 1);  // Send each byte of the drawing buffer over Serial1 (UART)
+        buffer[bytesToDrawInCurrentBlock] = '\n';
+        uart_write_blocking(uart0, buffer, bytesToDrawInCurrentBlock + 1);
     }
 
     bytesToDrawInCurrentBlock = 0;
@@ -290,6 +277,18 @@ void processReceivedByte(uint8_t bufferToRead) {
         return;
     }
 
+    if (!drawMode) {
+        if (dataByte >= ASCII_OFFSET + 18) {
+            Serial.println("Decoded byte is out of bounds for defined ROMAN_8_SYMBOLS.");
+        } else if (dataByte >= ASCII_OFFSET && dataByte < ASCII_OFFSET + 18) {
+            Serial.print("Decoded byte (roman symbol): ");
+            Serial.println(ROMAN_8_SYMBOLS[dataByte - ASCII_OFFSET]);
+        } else {
+            Serial.print("Decoded byte (ascii): ");
+            Serial.println((char)dataByte);
+        }
+    }
+
     uint_fast8_t calculateErrorCorrectionBits = calculateErrorCorrection(dataByte);  // Check error correction for the decoded byte;
 
     if (errorCorrectionBits != 255 && errorCorrectionBits != calculateErrorCorrectionBits) {