Improved code comments. Couple of vars renaming refactoring.

firmware/lib/DLUSB/DLUSB.cpp

@@ -14,35 +14,38 @@
  * License: GNU GPL v2 (see License.txt), GNU GPL v3 or proprietary (CommercialLicense.txt)
  * This Revision: $Id: main.c 692 2008-11-07 15:07:40Z cs $
  */
-
 #include <Arduino.h>
 #include <avr/io.h>
 #include <avr/wdt.h>
-#include <avr/interrupt.h>  /* for sei() */
-#include <util/delay.h>     /* for _delay_ms() */
+#include <avr/interrupt.h>  // for sei()
+#include <util/delay.h>     // for _delay_ms()
 #include <avr/eeprom.h>
-#include <avr/pgmspace.h>   /* required by usbdrv.h */
+#include <avr/pgmspace.h>   // required by usbdrv.h
 #include "usbdrv.h"
-#include "oddebug.h"        /* This is also an example for using debug macros */
+#include "oddebug.h"        // This is also an example for using debug macros
 
 #include "DLUSB.h"
 
- // Ring buffer implementation nicked from HardwareSerial.cpp
- // TODO: Don't nick it. :)
+/* Ring buffer implementation nicked from HardwareSerial.cpp
+ * TODO: Don't nick it. :) */
 ring_buffer rx_buffer = { { 0, 0, 0, 0 }, 0, 0 };
 ring_buffer tx_buffer = { { 0, 0, 0, 0 }, 0, 0 };
 
-inline bool store_packet(dlusb_packet_t* packet, ring_buffer* the_buffer)
+/// @brief Stores packet in the ring buffer.
+/// @param[in] packet stored packet struct
+/// @param[out] buffer pointer to a buffer struct
+/// @return true if success, false if buffer is full
+inline bool store_packet(dlusb_packet_t* packet, ring_buffer* buffer)
 {
-  uint8_t newhead = (the_buffer->head + 1) % RING_BUFFER_SIZE;
-
-  // if we should be storing the received data into the location
-  // just before the tail (meaning that the head would advance to the
-  // current location of the tail), we're about to overflow the buffer
-  // and so we don't write to the buffer or advance the head.
-  if (newhead != the_buffer->tail && newhead <= RING_BUFFER_SIZE) {
-    memcpy(&the_buffer->buffer[the_buffer->head], packet, sizeof(dlusb_packet_t));
-    the_buffer->head = newhead;
+  uint8_t newhead = (buffer->head + 1) % RING_BUFFER_SIZE;
+
+  /* if we should be storing the received data into the location
+   * just before the tail (meaning that the head would advance to the
+   * current location of the tail), we're about to overflow the buffer
+   * and so we don't write to the buffer or advance the head. */
+  if (newhead != buffer->tail && newhead <= RING_BUFFER_SIZE) {
+    memcpy(&buffer->buffer[buffer->head], packet, sizeof(dlusb_packet_t));
+    buffer->head = newhead;
     return true;
   }
   return false;
@@ -53,6 +56,7 @@ DLUSBDevice::DLUSBDevice(ring_buffer* rx_buffer, ring_buffer* tx_buffer) {
   _tx_buffer = tx_buffer;
 }
 
+/// @brief Initializes USB
 void DLUSBDevice::begin() {
   cli();
 
@@ -61,24 +65,26 @@ void DLUSBDevice::begin() {
   usbDeviceDisconnect();
   uchar   i;
   i = 0;
-  while (--i) {             /* fake USB disconnect for > 250 ms */
+  while (--i) { // fake USB disconnect for > 250 ms
     _delay_ms(10);
   }
   usbDeviceConnect();
 
   sei();
 }
 
+/// @brief Calls usbPoll() to process low-level USB stuff
 void DLUSBDevice::refresh() {
   usbPoll();
 }
 
-// wait a specified number of milliseconds (roughly), refreshing in the background
-void DLUSBDevice::delay(long milli) {
+/// @brief Wait a specified number of milliseconds (roughly), refreshing in the background
+/// @param[in] ms delay in milliseconds
+void DLUSBDevice::delay(long ms) {
   unsigned long last = millis();
-  while (milli > 0) {
+  while (ms > 0) {
     unsigned long now = millis();
-    milli -= now - last;
+    ms -= now - last;
     last = now;
     refresh();
   }
@@ -92,6 +98,9 @@ int DLUSBDevice::tx_remaining() {
   return RING_BUFFER_SIZE - (RING_BUFFER_SIZE + _tx_buffer->head - _tx_buffer->tail) % RING_BUFFER_SIZE;
 }
 
+/// @brief Returns next packet from rx_buffer
+/// @param packet[out] pointer to a struct where packet will be copied to
+/// @return true if success, false if there are no new packets in ring buffer available
 bool DLUSBDevice::read(dlusb_packet_t* packet) {
   // if the head isn't ahead of the tail, we don't have any characters
   if (_rx_buffer->head == _rx_buffer->tail) {
@@ -104,6 +113,9 @@ bool DLUSBDevice::read(dlusb_packet_t* packet) {
   }
 }
 
+/// @brief Stores packet to tx_buffer
+/// @param packet[in] pointer to a struct of packet to store
+/// @return true if success, false if buffer is full
 bool DLUSBDevice::write(dlusb_packet_t* packet) {
   return store_packet(packet, _tx_buffer);
 }
@@ -154,14 +166,15 @@ extern "C" {
   {
     usbRequest_t* rq = (usbRequest_t*)((void*)data);
 
-    if ((rq->bmRequestType & USBRQ_TYPE_MASK) == USBRQ_TYPE_CLASS) {    /* HID class request */
-      if (rq->bRequest == USBRQ_HID_GET_REPORT) {  /* wValue: ReportType (highbyte), ReportID (lowbyte) */
-        /* since we have only one report type, we can ignore the report-ID */
-        static dlusb_packet_t packet[1];  /* buffer must stay valid when usbFunctionSetup returns */
+    if ((rq->bmRequestType & USBRQ_TYPE_MASK) == USBRQ_TYPE_CLASS) {    // HID class request
+      // Host requests USB HID REPORT. Data: HOST <- DEVICE
+      if (rq->bRequest == USBRQ_HID_GET_REPORT) {  // wValue: ReportType (highbyte), ReportID (lowbyte)
+        // Since we have only one report type, we can ignore the report-ID
+        static dlusb_packet_t packet[1];  // Buffer must stay valid when usbFunctionSetup returns
         if (tx_available()) {
           if (tx_read(&packet[0])) {
-            usbMsgPtr = (unsigned char*)packet; /* tell the driver which data to return */
-            return sizeof(dlusb_packet_t); /* tell the driver to send packet */
+            usbMsgPtr = (unsigned char*)packet; // Tell the driver which data to return
+            return sizeof(dlusb_packet_t); // Tell the driver to send packet
           }
           else
             return 0;
@@ -170,19 +183,22 @@ extern "C" {
           // Drop through to return 0 (which will stall the request?)
         }
       }
+      // Host sets USB HID REPORT. Data: HOST -> DEVICE
       else if (rq->bRequest == USBRQ_HID_SET_REPORT) {
-        /* since we have only one report type, we can ignore the report-ID */
-        return USB_NO_MSG;  /* use usbFunctionWrite() to receive data from host */
+        // Since we have only one report type, we can ignore the report-ID
+        return USB_NO_MSG;  // use usbFunctionWrite() to receive data from host
       }
     }
     else {
-      /* ignore vendor type requests, we don't use any */
+      // Ignore vendor type requests, we don't use any
     }
     return 0;
   }
 
+  // Called when hosts sends data to device, i.e. device receives HID report
   uchar  usbFunctionWrite(uchar* data, uchar len)
   {
+    // Type cast incoming data to dlusb_packet_t struct
     dlusb_packet_t* p = (dlusb_packet_t*)data;
     // TODO: Check why len are not <= sizeof(dlusb_packet_t)
     if (p->report_id == REPORT_ID && p->cmd_id == CMD_SWITCH)

firmware/lib/DLUSB/DLUSB.h

@@ -12,22 +12,18 @@
 #include <string.h>
 #include <stdint.h>
 #include "usbdrv.h"
- // #include "Print.h"
 
- // typedef uint8_t byte;
-
-#include <util/delay.h>     /* for _delay_ms() */
+#include <util/delay.h> /* for _delay_ms() */
 
 /* Buffer size for USB TX & RX buffers. This is not the bytes, it's a count of dlusb_packet_t
-   structures it holds. I.e. how many packets it can store.
-   */
+ * structures it holds. I.e. how many packets it can store before processing. */
 #define RING_BUFFER_SIZE 16
 
 #define REPORT_ID 0x4c
 
-#define CMD_SWITCH 0x01
-#define CMD_RDY 0x10
-#define CMD_FAIL_BIT (uint8_t)(1 << 7)
+#define CMD_SWITCH 0x01 // IN,OUT send Livolo keycode command or send ACK to the host
+#define CMD_RDY 0x10 // OUT, device ready command
+#define CMD_FAIL_BIT (uint8_t)(1 << 7) // Not used
 
 typedef struct {
   uint8_t report_id;
@@ -42,6 +38,7 @@ struct ring_buffer {
   int tail;
 };
 
+/// @brief Class for interfacing with USB
 class DLUSBDevice {
 private:
   ring_buffer* _rx_buffer;

firmware/platformio.ini

@@ -1,19 +1,19 @@
-; PlatformIO Project Configuration File
-;
-;   Build options: build flags, source filter
-;   Upload options: custom upload port, speed and extra flags
-;   Library options: dependencies, extra library storages
-;   Advanced options: extra scripting
-;
-; Please visit documentation for the other options and examples
-; https://docs.platformio.org/page/projectconf.html
-
-[env:digispark-tiny]
-platform = atmelavr
-board = digispark-micronucleus-6586
-framework = arduino
-platform_packages = 
-	platformio/toolchain-atmelavr @ 3
-	framework-arduino-avr-digistump @ file://packages/framework-arduino-avr-digistump
-board_build.f_cpu = 16500000L
-build_flags = -Wmissing-field-initializers
+; PlatformIO Project Configuration File
+;
+;   Build options: build flags, source filter
+;   Upload options: custom upload port, speed and extra flags
+;   Library options: dependencies, extra library storages
+;   Advanced options: extra scripting
+;
+; Please visit documentation for the other options and examples
+; https://docs.platformio.org/page/projectconf.html
+
+[env:digispark-tiny]
+platform = atmelavr
+board = digispark-micronucleus-6586
+framework = arduino
+platform_packages = 
+	platformio/toolchain-atmelavr @ 3
+	framework-arduino-avr-digistump @ file://packages/framework-arduino-avr-digistump
+board_build.f_cpu = 16500000L
+build_flags = -Wmissing-field-initializers

firmware/src/DigiLivolo.cpp

@@ -1,44 +1,59 @@
-#include <Arduino.h>
-#include <DLUSB.h>
-#include <Livolo.h>
-#include <stdint.h>
-
-Livolo livolo(PIN_B5); // transmitter connected to pin #5
-dlusb_packet_t in_buf, out_buf;
-
-void prep_rdy_packet(dlusb_packet_t* packet) {
-  packet->report_id = REPORT_ID;
-  packet->cmd_id = CMD_RDY;
-
-  // Just some unused "magic" data to test connection below
-  packet->remote_id = 0xABCD;
-  packet->btn_id = 0xEF;
-}
-
-void setup() {
-  DLUSB.begin();
-  DLUSB.refresh();
-  prep_rdy_packet(&out_buf);
-  DLUSB.write(&out_buf);  
-  pinMode(LED_BUILTIN, OUTPUT);
-  digitalWrite(LED_BUILTIN, LOW);
-  DLUSB.refresh();
-}
-
-void loop() {
-  // Read data from host if available
-  if (DLUSB.available()) {
-    digitalWrite(LED_BUILTIN, HIGH);
-
-    if (DLUSB.read(&in_buf)) {
-      DLUSB.refresh();
-      livolo.sendButton(in_buf.remote_id, in_buf.btn_id);
-      DLUSB.refresh();
-      DLUSB.write(&in_buf);
-      DLUSB.delay(100);
-      digitalWrite(LED_BUILTIN, LOW);
-    }
-  }
-
-  DLUSB.delay(50);
-}
+#include <Arduino.h>
+#include <DLUSB.h>
+#include <Livolo.h>
+#include <stdint.h>
+
+Livolo livolo(PIN_B5); // Transmitter connected to pin #5
+dlusb_packet_t in_buf, out_buf; // Input & outpus USB packet buffers
+
+/// @brief Populates dlusb_packet_t struct with RDY packet which are sent
+///        to the host from setup() on device powerup/reset to signal the
+///        host software that the device are ready.
+/// @param packet[out] pointer to dlusb_packet_t struct
+void mk_rdy_packet(dlusb_packet_t* packet) {
+  packet->report_id = REPORT_ID;
+  packet->cmd_id = CMD_RDY;
+
+  // Just some unused "magic" data to test connection below.
+  packet->remote_id = 0xABCD;
+  packet->btn_id = 0xEF;
+}
+
+void setup() {
+  DLUSB.begin();
+  DLUSB.refresh();
+
+  /* Fill & send RDY packet to indicate device has been booted and ready to
+   * accept commands. */
+  mk_rdy_packet(&out_buf);
+  DLUSB.write(&out_buf);
+
+  pinMode(LED_BUILTIN, OUTPUT);
+  digitalWrite(LED_BUILTIN, LOW);
+  DLUSB.refresh();
+}
+
+void loop() {
+  // Read data from host if available.
+  if (DLUSB.available()) {
+    // Turn on the LED to indicate received packet.
+    digitalWrite(LED_BUILTIN, HIGH);
+
+    if (DLUSB.read(&in_buf)) {
+      DLUSB.refresh();
+      // Transmit Livolo code
+      livolo.sendButton(in_buf.remote_id, in_buf.btn_id);
+      DLUSB.refresh();
+      /* Send back same packet so that the host software can acknowledge it was
+       * processed by the device. */
+      DLUSB.write(&in_buf);
+      /* Sleep for 100ms after receiving packet & transmitting the keycode. We
+       * don't need to send codes more often. Makes LED blink noticable. */
+      DLUSB.delay(100);
+      digitalWrite(LED_BUILTIN, LOW); // LED off
+    }
+  }
+
+  // Sleep for 50ms between checking for packets from the host.
+  DLUSB.delay(50);
+}