📝 🥅 Applied additional change requests

include/libhal-soft/bit_bang_i2c.hpp

@@ -23,11 +23,15 @@ namespace hal {
 /**
  * @brief A bit bang implementation for i2c.
  *
- * This implementation of i2c only needs 2 gpios and a steady_clock to work
- * correctly. It supports single-controller multiple peripherals but  we intend
- * to support multiple controllers in the future. bit-bang is a best-effort
- * implementation of i2c. The maximum achievable clock rate for the lpc4078 is
- * about 180kHz. Interrupts disrupt this controller.
+ * This implementation of i2c only needs 2 hal::output_pins and a steady_clock
+ * to work. It does not support multi-controller but we intend to support it in
+ * the future. the data transfer rate for bit-bang is a best-effort
+ * implementation meaning it will almost always run at a frequency which is less
+ * then the request one but, never faster. The maximum achievable clock rate for
+ * the lpc4078 is about 180kHz. Interrupts disrupt this controller because the
+ * transfer is a blocking operation which means an interupt may come in the
+ * middle of a transaction and may leave a transaction hanging which some
+ * peripherals may not support.
  */
 class bit_bang_i2c : public i2c
 {
@@ -44,9 +48,10 @@ class bit_bang_i2c : public i2c
    * @param p_pins This holds both scl and sda to be used inside of the driver
    * @param p_steady_clock A steady clock that should have a higher frequency
    * then the configured frequency for the bit bang
-   * @param p_duty_cycle The duty cycle that the clock sent over scl will run at
+   * @param p_duty_cycle The duty cycle that the clock, sent over scl, will run
+   * at
    */
-  bit_bang_i2c(pins p_pins,
+  bit_bang_i2c(const pins& p_pins,
                steady_clock& p_steady_clock,
                const float p_duty_cycle = 0.5f);
 
@@ -171,7 +176,8 @@ class bit_bang_i2c : public i2c
   /// @brief The time that scl will be held low for
   uint64_t m_scl_low_ticks;
 
-  /// @brief All the information that the bus will need to operate on
+  /// @brief This is used to preserve the duty cycle that is passed in through
+  /// the constructor and be used in the driver_configure function
   float m_duty_cycle;
 };
 }  // namespace hal

src/bit_bang_i2c.cpp

@@ -32,7 +32,7 @@ namespace hal {
 }
 
 // Public
-bit_bang_i2c::bit_bang_i2c(pins p_pins,
+bit_bang_i2c::bit_bang_i2c(const pins& p_pins,
                            steady_clock& p_clock,
                            const float p_duty_cycle)
   : m_scl(p_pins.scl)
@@ -44,27 +44,28 @@ bit_bang_i2c::bit_bang_i2c(pins p_pins,
 
 // Private
 
+/*
+  It was decided that no calibration should be done to the calculation for ticks
+  in the configure function. In this context, calibration refers to the addition
+  of ticks to the high and low clock time, which are derived from the level
+  function of the output_pin and the uptime function of the steady_clock. This
+  decision was made because it would introduce two critical sections in the code
+  that the end user would have to deal with. Additionally, it would only improve
+  the accuracy by about 0.1 to 0.01 Hz per clock cycle. This marginal
+  improvement in accuracy didn't outweigh the potential drawbacks it would
+  introduce to the system. See libhal-soft/demos/seleae_captures for the
+  comparisons.
+*/
 void bit_bang_i2c::driver_configure(const settings& p_settings)
 {
   using namespace std::chrono_literals;
 
   if (p_settings.clock_rate > m_clock->frequency()) {
-    throw hal::operation_not_supported(this);
+    hal::safe_throw(hal::operation_not_supported(this));
   }
 
   using period = std::chrono::nanoseconds::period;
 
-  // Calculate the delay due to the uptime function call
-  const auto callibration_start_tick = m_clock->uptime();
-  const auto callibration_end_tick = m_clock->uptime();
-  const auto uptime_ticks = callibration_end_tick - callibration_start_tick;
-
-  // Calculating output_pin going to true delay time
-  const auto before_output = m_clock->uptime();
-  m_scl->level(true);
-  const auto after_output = m_clock->uptime();
-  const auto calibration_ticks = after_output - before_output - uptime_ticks;
-
   // Calculate period in nanosecond
   auto period_ns = hal::wavelength<period>(p_settings.clock_rate);
   auto scl_high_time = period_ns * m_duty_cycle;
@@ -74,11 +75,9 @@ void bit_bang_i2c::driver_configure(const settings& p_settings)
   const auto frequency = m_clock->frequency();
   const auto tick_period = hal::wavelength<period>(frequency);
 
-  m_scl_high_ticks =
-    static_cast<uint64_t>(scl_high_time / tick_period) - calibration_ticks;
-
-  m_scl_low_ticks =
-    static_cast<uint64_t>(scl_low_time / tick_period) - calibration_ticks;
+  // calculation for ticks
+  m_scl_high_ticks = static_cast<uint64_t>(scl_high_time / tick_period);
+  m_scl_low_ticks = static_cast<uint64_t>(scl_low_time / tick_period);
 }
 
 void bit_bang_i2c::driver_transaction(