"moving i2c over to callbacks; updates #1293"

software/controller/lib/core/v03system.cpp

@@ -171,6 +171,13 @@ void V03System::init_hal() {
   // every 100ns and generate an interrupt every millisecond. Note that this assumes the cpu
   // frequency is a multiple of 10MHz. Basic timers are documented in [RM] chapter 29.
   SystemTimer::singleton().initialize(PeripheralID::Timer6, InterruptVector::Timer6, CPUFrequency);
+
+#if defined(BARE_STM32)
+  hal.set_i2c1_event_callback(i2c1_.get_event_callback());
+  hal.set_i2c1_error_callback(i2c1_.get_error_callback());
+  hal.set_dma2ch6_callback(i2c1_.get_read_callback());
+  hal.set_dma2ch7_callback(i2c1_.get_write_callback());
+#endif
 }
 
 void V03System::init_subsystems() {
@@ -200,7 +207,8 @@ void V03System::init_subsystems() {
 
 #if defined(BARE_STM32)
   hal.set_uart2_callback(debug_uart_.get_callback());
-  hal.bind_channels(&i2c1_, &rpi_uart_);
+  //  hal.bind_channels(&i2c1_, &rpi_uart_);
+  hal.bind_channels(&rpi_uart_);
 #endif
 
   Interrupts::singleton().EnableInterrupts();

software/controller/lib/hal/hal.h

@@ -58,7 +58,13 @@ class HalApi {
 
 #if defined(BARE_STM32)
   void set_uart2_callback(Callback callback);
-  void bind_channels(I2C::Channel *i2c, UART::DMAChannel *rpi);
+  void set_i2c1_event_callback(Callback callback);
+  void set_i2c1_error_callback(Callback callback);
+  void set_dma2ch6_callback(Callback callback);
+  void set_dma2ch7_callback(Callback callback);
+
+  void bind_channels(UART::DMAChannel *rpi);
+  //  void bind_channels(I2C::Channel *i2c, UART::DMAChannel *rpi);
 
   // local static functions.  We don't want to add any private functions to the Hal class to avoid
   // complexity with other builds. Those are Interrupt Service Routines, i.e callback functions for
@@ -75,9 +81,13 @@ class HalApi {
   static void DMA2Channel7ISR();
 
  private:
-  I2C::Channel *i2c_{nullptr};
+  //  I2C::Channel *i2c_{nullptr};
   UART::DMAChannel *rpi_uart_{nullptr};
   Callback uart2_callback_;
+  Callback i2c1_event_callback_;
+  Callback i2c1_error_callback_;
+  Callback dma2ch6_callback_;
+  Callback dma2ch7_callback_;
 
 #endif
 

software/controller/lib/hal/hal_stm32.cpp

@@ -82,11 +82,17 @@ void HalApi::Init(Frequency cpu_frequency) {
 }
 
 void HalApi::set_uart2_callback(Callback callback) { uart2_callback_ = callback; }
+void HalApi::set_i2c1_event_callback(Callback callback) { i2c1_event_callback_ = callback; }
+void HalApi::set_i2c1_error_callback(Callback callback) { i2c1_error_callback_ = callback; }
+void HalApi::set_dma2ch6_callback(Callback callback) { dma2ch6_callback_ = callback; }
+void HalApi::set_dma2ch7_callback(Callback callback) { dma2ch7_callback_ = callback; }
 
-void HalApi::bind_channels(I2C::Channel *i2c, UART::DMAChannel *rpi) {
-  i2c_ = i2c;
-  rpi_uart_ = rpi;
-}
+// void HalApi::bind_channels(I2C::Channel *i2c, UART::DMAChannel *rpi) {
+//  i2c_ = i2c;
+//  rpi_uart_ = rpi;
+//}
+
+void HalApi::bind_channels(UART::DMAChannel *rpi) { rpi_uart_ = rpi; }
 
 // Reset the processor
 // this gets called from pure_virtual
@@ -145,27 +151,39 @@ void BadISR() {}
 // Those interrupt service routines are specific to our configuration, unlike
 // the I2C::Channel::*ISR() which are generic ISR associated with an I²C channel
 void HalApi::I2c1EventISR() {
-  if (hal.i2c_) {
-    hal.i2c_->I2CEventHandler();
+  if (hal.i2c1_event_callback_) {
+    hal.i2c1_event_callback_();
   }
+  //  if (hal.i2c_) {
+  //    hal.i2c_->I2CEventHandler();
+  //  }
 };
 
 void HalApi::I2c1ErrorISR() {
-  if (hal.i2c_) {
-    hal.i2c_->I2CErrorHandler();
+  if (hal.i2c1_error_callback_) {
+    hal.i2c1_error_callback_();
   }
+  //  if (hal.i2c_) {
+  //    hal.i2c_->I2CErrorHandler();
+  //  }
 };
 
 void HalApi::DMA2Channel6ISR() {
-  if (hal.i2c_) {
-    hal.i2c_->DMAInterruptHandler(I2C::ExchangeDirection::Read);
+  if (hal.dma2ch6_callback_) {
+    hal.dma2ch6_callback_();
   }
+  //  if (hal.i2c_) {
+  //    hal.i2c_->DMAInterruptHandler(I2C::ExchangeDirection::Read);
+  //  }
 };
 
 void HalApi::DMA2Channel7ISR() {
-  if (hal.i2c_) {
-    hal.i2c_->DMAInterruptHandler(I2C::ExchangeDirection::Write);
+  if (hal.dma2ch7_callback_) {
+    hal.dma2ch7_callback_();
   }
+  //  if (hal.i2c_) {
+  //    hal.i2c_->DMAInterruptHandler(I2C::ExchangeDirection::Write);
+  //  }
 };
 
 void HalApi::DMA1Channel2ISR() {

software/controller/lib/hal/i2c.cpp

@@ -250,6 +250,8 @@ void Channel::Initialize(Speed speed, GPIO::Port port, uint8_t scl_pin, uint8_t
     i2c_reg->control_reg1.tx_interrupts = 0;
     i2c_reg->control_reg1.tx_complete_interrupts = 0;
   }
+
+  initialized_ = true;
 }
 
 bool Channel::SendRequest(const Request &request) {
@@ -443,8 +445,53 @@ bool Channel::NackDetected() const { return get_register(i2c_)->status.nack; }
 void Channel::ClearNack() { get_register(i2c_)->interrupt_clear = 0x10; }
 void Channel::ClearErrors() { get_register(i2c_)->interrupt_clear = 0x720; }
 
+void Channel::SetupDMATransfer() {
+  if (!dma_enable_) {
+    return;
+  }
+  // to be on the safe size, disable both channels in case they weren't
+  // (likely when this is called as a "retry after error")
+  rx_dma_->Disable();
+  tx_dma_->Disable();
+
+  DMA::ChannelControl *channel{nullptr};
+  if (last_request_.direction == ExchangeDirection::Read) {
+    channel = &(rx_dma_.value());
+  } else {
+    channel = &(tx_dma_.value());
+  }
+
+  uint16_t transfer_size{0};
+  if (remaining_size_ <= 255) {
+    transfer_size = remaining_size_;
+  } else {
+    transfer_size = 255;
+  }
+
+  channel->SetupTransfer(next_data_, transfer_size);
+
+  // when using DMA, we need to use autoend, otherwise the STOP condition
+  // which we issue at the end of the DMA transfer (which means the last byte
+  // has been written to the register) may arrive before the last byte is
+  // actually written on the line. Tests with both DMA and I2C interrupts
+  // enabled to send Stop at the end of the I2C transfer were inconclusive.
+  get_register(i2c_)->control2.autoend = 1;
+
+  channel->Enable();
+}
+
+// Interrupt handlers
+
+Callback Channel::get_event_callback() {
+  return {static_cast<void *>(this), &static_event_handler};
+}
+
+void Channel::static_event_handler(void *instance) {
+  static_cast<Channel *>(instance)->I2CEventHandler();
+}
+
 void Channel::I2CEventHandler() {
-  if (!transfer_in_progress_) {
+  if (!initialized_ || !transfer_in_progress_) {
     return;
   }
 
@@ -485,43 +532,19 @@ void Channel::I2CEventHandler() {
   }
 }
 
-void Channel::SetupDMATransfer() {
-  if (!dma_enable_) {
-    return;
-  }
-  // to be on the safe size, disable both channels in case they weren't
-  // (likely when this is called as a "retry after error")
-  rx_dma_->Disable();
-  tx_dma_->Disable();
-
-  DMA::ChannelControl *channel{nullptr};
-  if (last_request_.direction == ExchangeDirection::Read) {
-    channel = &(rx_dma_.value());
-  } else {
-    channel = &(tx_dma_.value());
-  }
-
-  uint16_t transfer_size{0};
-  if (remaining_size_ <= 255) {
-    transfer_size = remaining_size_;
-  } else {
-    transfer_size = 255;
-  }
-
-  channel->SetupTransfer(next_data_, transfer_size);
-
-  // when using DMA, we need to use autoend, otherwise the STOP condition
-  // which we issue at the end of the DMA transfer (which means the last byte
-  // has been written to the register) may arrive before the last byte is
-  // actually written on the line. Tests with both DMA and I2C interrupts
-  // enabled to send Stop at the end of the I2C transfer were inconclusive.
-  get_register(i2c_)->control2.autoend = 1;
+Callback Channel::get_error_callback() {
+  return {static_cast<void *>(this), &static_error_handler};
+}
 
-  channel->Enable();
+void Channel::static_error_handler(void *instance) {
+  static_cast<Channel *>(instance)->I2CErrorHandler();
 }
 
-// Interrupt handlers
 void Channel::I2CErrorHandler() {
+  if (!initialized_) {
+    return;
+  }
+
   // I²C error --> clear all error flags (except those that are SMBus only)
   ClearErrors();
   // and restart the request up to error_retry_ times
@@ -535,8 +558,22 @@ void Channel::I2CErrorHandler() {
   StartTransfer();
 }
 
+Callback Channel::get_read_callback() { return {static_cast<void *>(this), &static_read_handler}; }
+
+Callback Channel::get_write_callback() {
+  return {static_cast<void *>(this), &static_write_handler};
+}
+
+void Channel::static_read_handler(void *instance) {
+  static_cast<Channel *>(instance)->DMAInterruptHandler(I2C::ExchangeDirection::Read);
+}
+
+void Channel::static_write_handler(void *instance) {
+  static_cast<Channel *>(instance)->DMAInterruptHandler(I2C::ExchangeDirection::Write);
+}
+
 void Channel::DMAInterruptHandler(ExchangeDirection direction) {
-  if (!dma_enable_ || !transfer_in_progress_) {
+  if (!initialized_ || !dma_enable_ || !transfer_in_progress_) {
     return;
   }
 

software/controller/lib/hal/i2c.h

@@ -19,6 +19,7 @@ limitations under the License.
 
 #pragma once
 
+#include "callback.h"
 #include "circular_buffer.h"
 #include "dma.h"
 #include "gpio.h"
@@ -100,9 +101,19 @@ class Channel {
   bool SendRequest(const Request &request);
 
   // Interrupt handlers
+  Callback get_event_callback();
+  static void static_event_handler(void *instance);
   void I2CEventHandler();
+
+  Callback get_error_callback();
+  static void static_error_handler(void *instance);
   void I2CErrorHandler();
+
   // Interrupt handler for DMA, which only makes sense on the STM32
+  Callback get_read_callback();
+  Callback get_write_callback();
+  static void static_read_handler(void *instance);
+  static void static_write_handler(void *instance);
   void DMAInterruptHandler(ExchangeDirection direction);
 
   // Queue of a few requests. The number of requests is arbitrary but
@@ -131,6 +142,7 @@ class Channel {
   uint16_t remaining_size_{0};
 
  private:
+  bool initialized_{false};
   Base i2c_;
   std::optional<DMA::ChannelControl> rx_dma_{std::nullopt};
   std::optional<DMA::ChannelControl> tx_dma_{std::nullopt};

software/controller/lib/hal/uart_soft.cpp

@@ -48,15 +48,14 @@ size_t SoftChannel::Write(uint8_t *buffer, size_t length, TxListener *txl) {
   return i;
 }
 
-void SoftChannel::static_callback_handler(void *instance) {
-  auto *self = static_cast<SoftChannel *>(instance);
-  self->UARTInterruptHandler();
-}
-
 Callback SoftChannel::get_callback() {
   return {static_cast<void *>(this), &static_callback_handler};
 }
 
+void SoftChannel::static_callback_handler(void *instance) {
+  static_cast<SoftChannel *>(instance)->UARTInterruptHandler();
+}
+
 void SoftChannel::UARTInterruptHandler() {
   // Process common interrupts
   Channel::UARTInterruptHandler();

software/controller/lib/hal/uart_soft.h

@@ -37,9 +37,9 @@ class SoftChannel : public Channel {
   void StopTx() override { tx_data_.Flush(); };
   void StopRx() override { rx_data_.Flush(); };
 
-  void UARTInterruptHandler() override;
   Callback get_callback();
   static void static_callback_handler(void *instance);
+  void UARTInterruptHandler() override;
 
  protected:
   // circular buffers for transmitting and receiving data