Code cleanup improvements from article writing

examples/cpp/driver_abstraction/core/driver/i2c.hpp

@@ -133,7 +133,7 @@ enum class pullups : uint8_t
 struct op_t
 {
 	/// Slave device address (7-bit).
-	uint8_t address{0};
+	addr_t address{0};
 
 	/// Operation to perform.
 	operation op{operation::stop};
@@ -274,10 +274,10 @@ class controller
 	controller& operator=(controller&&) = delete;
 
   public:
-  	/// Put the driver in an operational state.
+	/// Put the driver in an operational state, fully configured
 	virtual void start() noexcept = 0;
 
-	/// Put the driver in a non-operational state (e.g., powered off).
+	/// Put the driver in a non-operational state (powered off if possible).
 	virtual void stop() noexcept = 0;
 
 	/** Configure the I2C bus.
@@ -463,6 +463,8 @@ class controller
 	 * The base class handles the callback, so there is no need to worry about invoking
 	 * the callback from a client driver.
 	 *
+	 * This function must be asynchronous and not block.
+	 *
 	 * Just mark your callback `(void)callback` if you aren't using it.
 	 *
 	 * To indicate that a transfer has been enqueued for later processing, return

examples/cpp/driver_abstraction/core/driver/time_of_flight.hpp

@@ -113,10 +113,6 @@ enum class status
  */
 class sensor
 {
-  public:
-	/// ToF Callback function which retuns distance in mm
-	using read_cb_t = stdext::inplace_function<void(uint16_t)>;
-
   protected:
 	/** Default constructor.
 	 *
@@ -153,11 +149,13 @@ class sensor
 	~sensor() noexcept = default;
 
   public:
-  	/// Put the sensor into an operational state.
-	virtual void start() noexcept = 0;
+/// Turn on the device, put it into a fully operational state, and apply
+/// necessary configuration options.
+virtual void start() noexcept = 0;
 
-	/// Put the sensor into a non-operational state.
-	virtual void stop() noexcept = 0;
+/// Put the sensor into a non-operational state. If possible, the device should be powered down
+/// or put into the lowest power state.
+virtual void stop() noexcept = 0;
 
 	/** Check the maximum range in the dark.
 	 *
@@ -197,25 +195,27 @@ class sensor
 	 *
 	 * @param cb The functor which will be called when read() completes.
 	 */
-	virtual void registerReadCallback(const read_cb_t& cb) noexcept = 0;
+	virtual void registerReadCallback(const cb_t& cb) noexcept = 0;
 
 	/** Register a callback for the read() function.
 	 *
 	 * The read() function works asynchronously, and the result will be provided
 	 * to consumers through a callback function. When the read() operation completes,
-	 * the callback will be invoked with the result.
+	 * the callback will be invoked with the resulting range (in mm).
 	 *
 	 * This function must be implemented by the derived class.
 	 *
 	 * @param cb The functor which will be called when read() completes.
 	 */
-	virtual void registerReadCallback(read_cb_t&& cb) noexcept = 0;
+	virtual void registerReadCallback(cb_t&& cb) noexcept = 0;
 
 	/** Trigger a sensor read.
 	 *
 	 * Trigger an asynchronous read of the ToF sensor. The result will be provided
 	 * to consumers through a callback function. When the read() operation completes,
 	 * the callback will be invoked with the result.
+	 *
+	 * The sensor result will be presented as current range in mm.
 	 */
 	virtual void read() noexcept = 0;
 

examples/cpp/driver_abstraction/drivers/nrf52/nrf52_i2c_master.hpp

@@ -231,12 +231,12 @@ class nRFi2cController final : public embvm::i2c::controller
 		return pullups;
 	}
 
-	void enableInterrupts() noexcept final
+	void enableInterrupts() noexcept
 	{
 		nRFTWIMTranslator::enable_interrupts(TTWIIndex);
 	}
 
-	void disableInterrupts() noexcept final
+	void disableInterrupts() noexcept
 	{
 		nRFTWIMTranslator::disable_interrupts(TTWIIndex);
 	}

examples/cpp/driver_abstraction/drivers/st/vl53l1x/vl53l1x.cpp

@@ -334,8 +334,6 @@ void vl53l1x::readReg(const uint16_t* reg_buf, uint8_t* rx_buffer, size_t rx_siz
 	i2c_.transfer(t, cb);
 }
 
-// TODO: can't I do a write-read?
-// TODO: remove this stupid double byteswap! All the hardcoded valeus above are byteswapped!
 void vl53l1x::writeReg(uint16_t reg, const uint8_t* tx_buffer, size_t tx_size,
 					   const embvm::i2c::controller::cb_t& cb) noexcept
 {
@@ -381,7 +379,6 @@ void vl53l1x::readOscillatorCal() noexcept
 
 void vl53l1x::clearInterrupt() noexcept
 {
-	// TODO: byteswap?
 	auto* int_clear = create(VL53L1_INT_CLR);
 	writeReg(SYSTEM_INTERUPT_CLEAR, int_clear, sizeof(uint16_t), [&](auto op, auto status) {
 		(void)op;
@@ -393,8 +390,6 @@ void vl53l1x::clearInterrupt() noexcept
 
 void vl53l1x::readTrim() noexcept
 {
-	// TODO: I hate this allocation scheme with this kind of iteration... Can we read a block?
-	// (Naive block read didn't work)
 	// Constants lifted from Sparkfun's Arduino code
 
 	for(uint16_t i = 1; i < VL53L1X_TRIM_BYTE_COUNT; i++)

examples/cpp/driver_abstraction/drivers/st/vl53l1x/vl53l1x.hpp

@@ -8,9 +8,7 @@
 #include <etl/list.h>
 #include <etl/pool.h>
 
-// TODO: make this a configuration option that we get from the build system
 #define ENABLE_THREADING
-
 #ifdef ENABLE_THREADING
 #include <mutex>
 #define VL53L1X_LOCK() lock_.lock()
@@ -20,10 +18,6 @@
 #define VL53L1X_LOCK()
 #endif
 
-// Reference for future improvements:
-// https://github.com/pololu/vl53l1x-st-api-arduino/tree/controller/vl53l1x-st-api
-// TODO: how can I make this queue size customizable without having template hell?
-
 namespace embdrv
 {
 /// Default I2C address of the VL53L1X part.
@@ -101,12 +95,12 @@ class vl53l1x final : public embvm::tof::sensor
 	// setZoneSize(uint8_t width, uint8_t height) // size of custom zone
 	// roi* getUserROI
 
-	void registerReadCallback(const read_cb_t& cb) noexcept final
+	void registerReadCallback(const embvm::tof::cb_t& cb) noexcept final
 	{
 		read_cb_list_.insert(read_cb_list_.end(), cb);
 	}
 
-	void registerReadCallback(read_cb_t&& cb) noexcept final
+	void registerReadCallback(embvm::tof::cb_t&& cb) noexcept final
 	{
 		read_cb_list_.insert(read_cb_list_.end(), cb);
 	}
@@ -233,7 +227,7 @@ class vl53l1x final : public embvm::tof::sensor
 #endif
 
 	/// List that stores the read() callback functions.
-	etl::list<read_cb_t, 2> read_cb_list_{};
+	etl::list<embvm::tof::cb_t, 2> read_cb_list_{};
 
 	/// List that stores the rangeStatus() callback functions.
 	etl::list<status_cb_t, 2> status_cb_list_{};
@@ -268,102 +262,6 @@ class vl53l1x final : public embvm::tof::sensor
 		0x0D, 0x0E, 0x0E, 0x01, 0x00, 0x02, 0xC7, 0xFF, // 128
 		0x8B, 0x00, 0x00, 0x00, 0x01, 0x01, 0x40 // 129 - 135 (0x81 - 0x87)
 	};
-
-#if 0
-const uint8_t VL51L1X_DEFAULT_CONFIGURATION[] = {
-0x00, /* 0x2d : set bit 2 and 5 to 1 for fast plus mode (1MHz I2C), else don't touch */
-0x00, /* 0x2e : bit 0 if I2C pulled up at 1.8V, else set bit 0 to 1 (pull up at AVDD) */
-0x00, /* 0x2f : bit 0 if GPIO pulled up at 1.8V, else set bit 0 to 1 (pull up at AVDD) */
-0x01, /* 0x30 : set bit 4 to 0 for active high interrupt and 1 for active low (bits 3:0 must be 0x1), use SetInterruptPolarity() */
-0x02, /* 0x31 : bit 1 = interrupt depending on the polarity, use CheckForDataReady() */
-0x00, /* 0x32 : not user-modifiable */
-0x02, /* 0x33 : not user-modifiable */
-0x08, /* 0x34 : not user-modifiable */
-0x00, /* 0x35 : not user-modifiable */
-0x08, /* 0x36 : not user-modifiable */
-0x10, /* 0x37 : not user-modifiable */
-0x01, /* 0x38 : not user-modifiable */
-0x01, /* 0x39 : not user-modifiable */
-0x00, /* 0x3a : not user-modifiable */
-0x00, /* 0x3b : not user-modifiable */
-0x00, /* 0x3c : not user-modifiable */
-0x00, /* 0x3d : not user-modifiable */
-0xff, /* 0x3e : not user-modifiable */
-0x00, /* 0x3f : not user-modifiable */
-0x0F, /* 0x40 : not user-modifiable */
-0x00, /* 0x41 : not user-modifiable */
-0x00, /* 0x42 : not user-modifiable */
-0x00, /* 0x43 : not user-modifiable */
-0x00, /* 0x44 : not user-modifiable */
-0x00, /* 0x45 : not user-modifiable */
-0x20, /* 0x46 : interrupt configuration 0->level low detection, 1-> level high, 2-> Out of window, 3->In window, 0x20-> New sample ready , TBC */
-0x0b, /* 0x47 : not user-modifiable */
-0x00, /* 0x48 : not user-modifiable */
-0x00, /* 0x49 : not user-modifiable */
-0x02, /* 0x4a : not user-modifiable */
-0x0a, /* 0x4b : not user-modifiable */
-0x21, /* 0x4c : not user-modifiable */
-0x00, /* 0x4d : not user-modifiable */
-0x00, /* 0x4e : not user-modifiable */
-0x05, /* 0x4f : not user-modifiable */
-0x00, /* 0x50 : not user-modifiable */
-0x00, /* 0x51 : not user-modifiable */
-0x00, /* 0x52 : not user-modifiable */
-0x00, /* 0x53 : not user-modifiable */
-0xc8, /* 0x54 : not user-modifiable */
-0x00, /* 0x55 : not user-modifiable */
-0x00, /* 0x56 : not user-modifiable */
-0x38, /* 0x57 : not user-modifiable */
-0xff, /* 0x58 : not user-modifiable */
-0x01, /* 0x59 : not user-modifiable */
-0x00, /* 0x5a : not user-modifiable */
-0x08, /* 0x5b : not user-modifiable */
-0x00, /* 0x5c : not user-modifiable */
-0x00, /* 0x5d : not user-modifiable */
-0x01, /* 0x5e : not user-modifiable */
-0xdb, /* 0x5f : not user-modifiable */
-0x0f, /* 0x60 : not user-modifiable */
-0x01, /* 0x61 : not user-modifiable */
-0xf1, /* 0x62 : not user-modifiable */
-0x0d, /* 0x63 : not user-modifiable */
-0x01, /* 0x64 : Sigma threshold MSB (mm in 14.2 format for MSB+LSB), use SetSigmaThreshold(), default value 90 mm  */
-0x68, /* 0x65 : Sigma threshold LSB */
-0x00, /* 0x66 : Min count Rate MSB (MCPS in 9.7 format for MSB+LSB), use SetSignalThreshold() */
-0x80, /* 0x67 : Min count Rate LSB */
-0x08, /* 0x68 : not user-modifiable */
-0xb8, /* 0x69 : not user-modifiable */
-0x00, /* 0x6a : not user-modifiable */
-0x00, /* 0x6b : not user-modifiable */
-0x00, /* 0x6c : Intermeasurement period MSB, 32 bits register, use SetIntermeasurementInMs() */
-0x00, /* 0x6d : Intermeasurement period */
-0x0f, /* 0x6e : Intermeasurement period */
-0x89, /* 0x6f : Intermeasurement period LSB */
-0x00, /* 0x70 : not user-modifiable */
-0x00, /* 0x71 : not user-modifiable */
-0x00, /* 0x72 : distance threshold high MSB (in mm, MSB+LSB), use SetD:tanceThreshold() */
-0x00, /* 0x73 : distance threshold high LSB */
-0x00, /* 0x74 : distance threshold low MSB ( in mm, MSB+LSB), use SetD:tanceThreshold() */
-0x00, /* 0x75 : distance threshold low LSB */
-0x00, /* 0x76 : not user-modifiable */
-0x01, /* 0x77 : not user-modifiable */
-0x0f, /* 0x78 : not user-modifiable */
-0x0d, /* 0x79 : not user-modifiable */
-0x0e, /* 0x7a : not user-modifiable */
-0x0e, /* 0x7b : not user-modifiable */
-0x00, /* 0x7c : not user-modifiable */
-0x00, /* 0x7d : not user-modifiable */
-0x02, /* 0x7e : not user-modifiable */
-0xc7, /* 0x7f : ROI center, use SetROI() */
-0xff, /* 0x80 : XY ROI (X=Width, Y=Height), use SetROI() */
-0x9B, /* 0x81 : not user-modifiable */
-0x00, /* 0x82 : not user-modifiable */
-0x00, /* 0x83 : not user-modifiable */
-0x00, /* 0x84 : not user-modifiable */
-0x01, /* 0x85 : not user-modifiable */
-0x00, /* 0x86 : clear interrupt, use ClearInterrupt() */
-0x00  /* 0x87 : start ranging, use StartRanging() or StopRanging(), If you want an automatic start after VL53L1X_init() call, put 0x40 in location 0x87 */
-};
-#endif
 };
 
 } // namespace embdrv