Remove unnecessary functions and simplify comments

wii-ext/src/async_impl.rs

@@ -1,6 +1,6 @@
-/// Async classic controller code
+/// Async classic controller driver
 pub mod classic;
 /// Async i2c interface code
 pub mod interface;
-
+/// Async nunchuk controller driver
 pub mod nunchuk;

wii-ext/src/async_impl/classic.rs

@@ -1,18 +1,6 @@
-// See https://www.raphnet-tech.com/support/classic_controller_high_res/ for data on high-precision mode
-
-// Abridged version of the above:
-// To enable High Resolution Mode, you simply write 0x03 to address 0xFE in the extension controller memory.
-// Then you poll the controller by reading 8 bytes at address 0x00 instead of only 6.
-// You can also restore the original format by writing the original value back to address 0xFE at any time.
-//
-// Classic mode:
-// http://wiibrew.org/wiki/Wiimote/Extension_Controllers/Classic_Controller
-//
-// See `decode_classic_report` and `decode_classic_hd_report` for data format
-
 use crate::async_impl::interface::{AsyncImplError, InterfaceAsync};
 use crate::core::classic::*;
-use crate::core::{ControllerIdReport, ControllerType};
+use crate::core::ControllerType;
 use embedded_hal_async;
 
 pub struct ClassicAsync<I2C, Delay> {
@@ -26,11 +14,7 @@ where
     I2C: embedded_hal_async::i2c::I2c,
     Delay: embedded_hal_async::delay::DelayNs,
 {
-    /// Create a new Wii Nunchuck
-    ///
-    /// This method will open the provide i2c device file and will
-    /// send the required init sequence in order to read data in
-    /// the future.
+    /// Create a new Wii Classic Controller
     pub fn new(i2cdev: I2C, delay: Delay) -> Self {
         let interface = InterfaceAsync::new(i2cdev, delay);
         Self {
@@ -40,15 +24,15 @@ where
         }
     }
 
-    /// Recover data members
+    /// Destroy this driver, recovering the i2c bus and delay used to create it
     pub fn destroy(self) -> (I2C, Delay) {
         self.interface.destroy()
     }
 
-    // / Update the stored calibration for this controller
-    // /
-    // / Since each device will have different tolerances, we take a snapshot of some analog data
-    // / to use as the "baseline" center.
+    /// Update the stored calibration for this controller
+    ///
+    /// Since each device will have different tolerances, we take a snapshot of some analog data
+    /// to use as the "baseline" center.
     pub async fn update_calibration(&mut self) -> Result<(), AsyncImplError> {
         let data = self.read_report().await?;
         self.calibration = CalibrationData {
@@ -62,23 +46,15 @@ where
         Ok(())
     }
 
-    /// Send the init sequence to the Wii extension controller
-    ///
-    /// This could be a bit faster with DelayUs, but since you only init once we'll re-use delay_ms
+    /// Send the init sequence to the controller and calibrate it
     pub async fn init(&mut self) -> Result<(), AsyncImplError> {
-        // Extension controllers by default will use encrypted communication, as that is what the Wii does.
-        // We can disable this encryption by writing some magic values
-        // This is described at https://wiibrew.org/wiki/Wiimote/Extension_Controllers#The_New_Way
-
-        // Reset to base register first - this should recover a controller in a weird state.
-        // Use longer delays here than normal reads - the system seems more unreliable performing these commands
         self.interface.init().await?;
         self.update_calibration().await?;
         Ok(())
     }
 
-    /// poll the controller for the latest data
-    async fn read_classic_report(&mut self) -> Result<ClassicReading, AsyncImplError> {
+    /// Read uncalibrated data from the controller
+    async fn read_report(&mut self) -> Result<ClassicReading, AsyncImplError> {
         if self.hires {
             let buf = self.interface.read_hd_report().await?;
             ClassicReading::from_data(&buf).ok_or(AsyncImplError::InvalidInputData)
@@ -88,27 +64,24 @@ where
         }
     }
 
-    /// Simple blocking read helper that will start a sample, wait 10ms, then read the value
-    async fn read_report(&mut self) -> Result<ClassicReading, AsyncImplError> {
-        self.read_classic_report().await
-    }
-
     /// Do a read, and report axis values relative to calibration
     pub async fn read(&mut self) -> Result<ClassicReadingCalibrated, AsyncImplError> {
         Ok(ClassicReadingCalibrated::new(
-            self.read_classic_report().await?,
+            self.read_report().await?,
             &self.calibration,
         ))
     }
 
+    /// Switch the driver from standard to hi-resolution reporting
+    ///
+    /// This enables the controllers high-resolution report data mode, which returns each
+    /// analogue axis as a u8, rather than packing smaller integers in a structure.
+    /// If your controllers supports this mode, you should use it. It is much better.
     pub async fn enable_hires(&mut self) -> Result<(), AsyncImplError> {
         self.interface.enable_hires().await
     }
 
-    pub async fn read_id(&mut self) -> Result<ControllerIdReport, AsyncImplError> {
-        self.interface.read_id().await
-    }
-
+    /// Determine the controller type based on the type ID of the extension controller
     pub async fn identify_controller(&mut self) -> Result<Option<ControllerType>, AsyncImplError> {
         self.interface.identify_controller().await
     }

wii-ext/src/async_impl/interface.rs

@@ -1,15 +1,3 @@
-// See https://www.raphnet-tech.com/support/classic_controller_high_res/ for data on high-precision mode
-
-// Abridged version of the above:
-// To enable High Resolution Mode, you simply write 0x03 to address 0xFE in the extension controller memory.
-// Then you poll the controller by reading 8 bytes at address 0x00 instead of only 6.
-// You can also restore the original format by writing the original value back to address 0xFE at any time.
-//
-// Classic mode:
-// http://wiibrew.org/wiki/Wiimote/Extension_Controllers/Classic_Controller
-//
-// See `decode_classic_report` and `decode_classic_hd_report` for data format
-
 use crate::core::{
     ControllerIdReport, ControllerType, ExtHdReport, ExtReport, EXT_I2C_ADDR,
     INTERMESSAGE_DELAY_MICROSEC_U32,
@@ -33,31 +21,27 @@ pub struct InterfaceAsync<I2C, Delay> {
     delay: Delay,
 }
 
-// use crate::nunchuk;
 impl<I2C, Delay> InterfaceAsync<I2C, Delay>
 where
     I2C: embedded_hal_async::i2c::I2c,
     Delay: embedded_hal_async::delay::DelayNs,
 {
-    /// Create a new Wii Nunchuck
-    ///
-    /// This method will open the provide i2c device file and will
-    /// send the required init sequence in order to read data in
-    /// the future.
+    /// Create async interface for wii-extension controller
     pub fn new(i2cdev: I2C, delay: Delay) -> Self {
         Self { i2cdev, delay }
     }
 
-    /// Recover data members
+    /// Destroy i2c interface, allowing recovery of i2c and delay
     pub fn destroy(self) -> (I2C, Delay) {
         (self.i2cdev, self.delay)
     }
 
+    /// Access delay stored in interface
     pub(super) async fn delay_us(&mut self, micros: u32) {
         self.delay.delay_us(micros).await
     }
 
-    /// Read the button/axis data from the classic controller
+    /// Read report data from the wii-extension controller
     pub(super) async fn read_ext_report(&mut self) -> Result<ExtReport, AsyncImplError> {
         self.start_sample().await?;
         self.delay_us(INTERMESSAGE_DELAY_MICROSEC_U32).await;
@@ -69,7 +53,7 @@ where
             .and(Ok(buffer))
     }
 
-    /// Read a high-resolution version of the button/axis data from the classic controller
+    /// Read a high-resolution version of the report data from the wii-extension controller
     pub(super) async fn read_hd_report(&mut self) -> Result<ExtHdReport, AsyncImplError> {
         self.start_sample().await?;
         self.delay_us(INTERMESSAGE_DELAY_MICROSEC_U32).await;
@@ -82,8 +66,6 @@ where
     }
 
     /// Send the init sequence to the Wii extension controller
-    ///
-    /// This could be a bit faster with DelayUs, but since you only init once we'll re-use delay_ms
     pub(super) async fn init(&mut self) -> Result<(), AsyncImplError> {
         // Extension controllers by default will use encrypted communication, as that is what the Wii does.
         // We can disable this encryption by writing some magic values
@@ -101,7 +83,7 @@ where
 
     /// Switch the driver from standard to hi-resolution reporting
     ///
-    /// This enables the controllers high-resolution report data mode, which returns each
+    /// This enables the controller's high-resolution report data mode, which returns each
     /// analogue axis as a u8, rather than packing smaller integers in a structure.
     /// If your controllers supports this mode, you should use it. It is much better.
     pub(super) async fn enable_hires(&mut self) -> Result<(), AsyncImplError> {
@@ -127,6 +109,13 @@ where
             .and(Ok(()))
     }
 
+    /// Set the cursor position for the next i2c read after a small delay
+    ///
+    /// This hardware has a range of 100 registers and automatically
+    /// increments the register read postion on each read operation, and also on
+    /// every write operation.
+    /// This should be called before a read operation to ensure you get the correct data
+    /// The delay helps ensure that required timings are met
     pub(super) async fn set_read_register_address_with_delay(
         &mut self,
         byte0: u8,
@@ -145,6 +134,7 @@ where
             .and(Ok(()))
     }
 
+    /// Set a single register at target address after a small delay
     pub(super) async fn set_register_with_delay(
         &mut self,
         addr: u8,
@@ -155,20 +145,22 @@ where
         res.await
     }
 
+    /// Read the controller type ID register from the extension controller
     pub(super) async fn read_id(&mut self) -> Result<ControllerIdReport, AsyncImplError> {
         self.set_read_register_address(0xfa).await?;
         let i2c_id = self.read_ext_report().await?;
         Ok(i2c_id)
     }
 
+    /// Determine the controller type based on the type ID of the extension controller
     pub(super) async fn identify_controller(
         &mut self,
     ) -> Result<Option<ControllerType>, AsyncImplError> {
         let i2c_id = self.read_id().await?;
         Ok(crate::core::identify_controller(i2c_id))
     }
 
-    /// tell the extension controller to prepare a sample by setting the read cursor to 0
+    /// Instruct the extension controller to start preparing a sample by setting the read cursor to 0
     pub(super) async fn start_sample(&mut self) -> Result<(), AsyncImplError> {
         self.set_read_register_address(0x00).await?;
         Ok(())

wii-ext/src/async_impl/nunchuk.rs

@@ -1,14 +1,6 @@
-// The nunchuk portion of this crate is derived from
-// https://github.com/rust-embedded/rust-i2cdev/blob/master/examples/nunchuck.rs
-// which is Copyright 2015, Paul Osborne <[email protected]>
-//
-// All the bugs are Copyright 2024, 9names.
-
-// Nunchuk technically supports HD report, but the last two bytes will be zeroes
-// There's no benefit, so we're leaving that unimplemented
 use crate::async_impl::interface::{AsyncImplError, InterfaceAsync};
 use crate::core::nunchuk::*;
-use crate::core::{ControllerIdReport, ControllerType};
+use crate::core::ControllerType;
 use embedded_hal_async;
 
 pub struct NunchukAsync<I2C, Delay> {
@@ -22,10 +14,6 @@ where
     Delay: embedded_hal_async::delay::DelayNs,
 {
     /// Create a new Wii Nunchuck
-    ///
-    /// This method will open the provide i2c device file and will
-    /// send the required init sequence in order to read data in
-    /// the future.
     pub fn new(i2cdev: I2C, delay: Delay) -> Self {
         let interface = InterfaceAsync::new(i2cdev, delay);
         Self {
@@ -34,7 +22,7 @@ where
         }
     }
 
-    /// Recover data members
+    /// Destroy this driver, recovering the i2c bus and delay used to create it
     pub fn destroy(self) -> (I2C, Delay) {
         self.interface.destroy()
     }
@@ -52,16 +40,8 @@ where
         Ok(())
     }
 
-    /// Send the init sequence to the Wii extension controller
-    ///
-    /// This could be a bit faster with DelayUs, but since you only init once we'll re-use delay_ms
+    /// Send the init sequence to the controller and calibrate it
     pub async fn init(&mut self) -> Result<(), AsyncImplError> {
-        // Extension controllers by default will use encrypted communication, as that is what the Wii does.
-        // We can disable this encryption by writing some magic values
-        // This is described at https://wiibrew.org/wiki/Wiimote/Extension_Controllers#The_New_Way
-
-        // Reset to base register first - this should recover a controller in a weird state.
-        // Use longer delays here than normal reads - the system seems more unreliable performing these commands
         self.interface.init().await?;
         self.update_calibration().await?;
         Ok(())
@@ -81,10 +61,7 @@ where
         ))
     }
 
-    pub async fn read_id(&mut self) -> Result<ControllerIdReport, AsyncImplError> {
-        self.interface.read_id().await
-    }
-
+    /// Determine the controller type based on the type ID of the extension controller
     pub async fn identify_controller(&mut self) -> Result<Option<ControllerType>, AsyncImplError> {
         self.interface.identify_controller().await
     }