Updating code after review feedback

Cargo.lock

@@ -237,12 +237,9 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 [[package]]
 name = "mcp23017"
 version = "0.1.1"
-source = "registry+https://github.com/rust-lang/crates.io-index"
+source = "git+https://github.com/mrd0ll4r/mcp23017.git#a3d072754abca60a92ece820f7cfb767a0c11669"
 dependencies = [
- "cast 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "embedded-hal 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)",
- "generic-array 0.13.2 (registry+https://github.com/rust-lang/crates.io-index)",
- "nb 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
@@ -372,7 +369,7 @@ dependencies = [
  "embedded-hal 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "heapless 0.5.4 (registry+https://github.com/rust-lang/crates.io-index)",
  "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)",
- "mcp23017 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
+ "mcp23017 0.1.1 (git+https://github.com/mrd0ll4r/mcp23017.git)",
  "nb 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "panic-halt 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "panic-semihosting 0.5.3 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -508,7 +505,7 @@ dependencies = [
 "checksum indexmap 1.3.2 (registry+https://github.com/rust-lang/crates.io-index)" = "076f042c5b7b98f31d205f1249267e12a6518c1481e9dae9764af19b707d2292"
 "checksum log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)" = "14b6052be84e6b71ab17edffc2eeabf5c2c3ae1fdb464aae35ac50c67a44e1f7"
 "checksum managed 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)" = "fdcec5e97041c7f0f1c5b7d93f12e57293c831c646f4cc7a5db59460c7ea8de6"
-"checksum mcp23017 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "7f4854484a74b626165c3f3cf5e15bfee2898c6b4d6eccc3ed5ee634850af4dd"
+"checksum mcp23017 0.1.1 (git+https://github.com/mrd0ll4r/mcp23017.git)" = "<none>"
 "checksum nb 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "b1411551beb3c11dedfb0a90a0fa256b47d28b9ec2cdff34c25a2fa59e45dbdc"
 "checksum panic-halt 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "de96540e0ebde571dc55c73d60ef407c653844e6f9a1e2fdbd40c07b9252d812"
 "checksum panic-semihosting 0.5.3 (registry+https://github.com/rust-lang/crates.io-index)" = "c03864ac862876c16a308f5286f4aa217f1a69ac45df87ad3cd2847f818a642c"

Cargo.toml

@@ -39,7 +39,9 @@ cortex-m-rtfm = "0.5"
 embedded-hal = "0.2.3"
 nb = "0.1.2"
 asm-delay = "0.7.0"
-mcp23017 = "0.1.1"
+
+[dependencies.mcp23017]
+git = "https://github.com/mrd0ll4r/mcp23017.git"
 
 [dependencies.smoltcp]
 version = "0.6"

src/afe.rs

@@ -30,23 +30,16 @@ where
     }
 
     pub fn set_gain(&mut self, gain: Gain) {
-        match gain {
+        if (gain as u8 & 0b01) != 0 {
-            Gain::G1 => {
+            self.a0.set_high().unwrap();
-                self.a0.set_low().unwrap();
+        } else {
-                self.a1.set_low().unwrap();
+            self.a0.set_low().unwrap();
-            },
+        }
-            Gain::G2 => {
+
-                self.a0.set_high().unwrap();
+        if (gain as u8 & 0b10) != 0 {
-                self.a1.set_low().unwrap();
+            self.a1.set_high().unwrap()
-            },
+        } else {
-            Gain::G5 => {
+            self.a1.set_low().unwrap();
-                self.a0.set_low().unwrap();
-                self.a1.set_high().unwrap();
-            },
-            Gain::G10 => {
-                self.a0.set_high().unwrap();
-                self.a1.set_high().unwrap();
-            },
         }
     }
 

src/pounder/attenuators.rs

@@ -3,18 +3,23 @@ use super::DdsChannel;
 
 pub trait AttenuatorInterface {
     fn modify(&mut self, attenuation: f32, channel: DdsChannel) -> Result<f32, Error> {
-        if attenuation > 31.5 {
+        if attenuation > 31.5 || attenuation < 0.0 {
             return Err(Error::Bounds);
         }
 
-        // Calculate the attenuation code to program into the attenuator.
+        // Calculate the attenuation code to program into the attenuator. The attenuator uses a
+        // code where the LSB is 0.5 dB.
         let attenuation_code = (attenuation * 2.0) as u8;
 
         // Read all the channels, modify the channel of interest, and write all the channels back.
         // This ensures the staging register and the output register are always in sync.
         let mut channels = [0_u8; 4];
         self.read_all(&mut channels)?;
-        channels[channel as usize] = attenuation_code;
+
+        // The lowest 2 bits of the 8-bit shift register on the attenuator are ignored. Shift the
+        // attenuator code into the upper 6 bits of the register value. Note that the attenuator
+        // treats inputs as active-low, so the code is inverted before writing.
+        channels[channel as usize] = !attenuation_code.wrapping_shl(2);
         self.write_all(&channels)?;
 
         // Finally, latch the output of the updated channel to force it into an active state.
@@ -32,9 +37,15 @@ pub trait AttenuatorInterface {
         self.read_all(&mut channels)?;
         self.write_all(&channels)?;
 
-        // Convert the desired channel code into dB of attenuation.
+        // The attenuation code is stored in the upper 6 bits of the register, where each LSB
-        let attenuation_code = channels[channel as usize];
+        // represents 0.5 dB. The attenuator stores the code as active-low, so inverting the result
+        // (before the shift) has the affect of transforming the bits of interest (and the
+        // dont-care bits) into an active-high state and then masking off the don't care bits. If
+        // the shift occurs before the inversion, the upper 2 bits (which would then be don't
+        // care) would contain erroneous data.
+        let attenuation_code = (!channels[channel as usize]).wrapping_shr(2);
 
+        // Convert the desired channel code into dB of attenuation.
         Ok(attenuation_code as f32 / 2.0)
     }
 

src/pounder/mod.rs

@@ -103,8 +103,9 @@ where
 
         // Configure power-on-default state for pounder. All LEDs are on, on-board oscillator
         // selected, attenuators out of reset.
-        devices.mcp23017.write_gpioa(0xF).map_err(|_| Error::I2c)?;
+        devices.mcp23017.write_gpio(mcp23017::Port::GPIOA, 0xF).map_err(|_| Error::I2c)?;
-        devices.mcp23017.write_gpiob(1_u8.wrapping_shl(5)).map_err(|_| Error::I2c)?;
+        devices.mcp23017.write_gpio(mcp23017::Port::GPIOB,
+                                    1_u8.wrapping_shl(5)).map_err(|_| Error::I2c)?;
         devices.mcp23017.all_pin_mode(mcp23017::PinMode::OUTPUT).map_err(|_| Error::I2c)?;
 
         devices.select_onboard_clock()?;
@@ -113,14 +114,14 @@ where
     }
 
     pub fn select_external_clock(&mut self, frequency: u32) -> Result<(), Error>{
-        self.mcp23017.digital_write(EXT_CLK_SEL_PIN, 1).map_err(|_| Error::I2c)?;
+        self.mcp23017.digital_write(EXT_CLK_SEL_PIN, true).map_err(|_| Error::I2c)?;
         self.ad9959.set_clock_frequency(frequency).map_err(|_| Error::DDS)?;
 
         Ok(())
     }
 
     pub fn select_onboard_clock(&mut self) -> Result<(), Error> {
-        self.mcp23017.digital_write(EXT_CLK_SEL_PIN, 0).map_err(|_| Error::I2c)?;
+        self.mcp23017.digital_write(EXT_CLK_SEL_PIN, false).map_err(|_| Error::I2c)?;
         self.ad9959.set_clock_frequency(100_000_000).map_err(|_| Error::DDS)?;
 
         Ok(())
@@ -130,24 +131,26 @@ where
 impl<DELAY> AttenuatorInterface for PounderDevices<DELAY>
 {
     fn reset(&mut self) -> Result<(), Error> {
-        self.mcp23017.digital_write(ATT_RST_N_PIN, 1).map_err(|_| Error::I2c)?;
+        self.mcp23017.digital_write(ATT_RST_N_PIN, true).map_err(|_| Error::I2c)?;
-        // TODO: Delay here.
+        // TODO: Measure the I2C transaction speed to the RST pin to ensure that the delay is
-        self.mcp23017.digital_write(ATT_RST_N_PIN, 0).map_err(|_| Error::I2c)?;
+        // sufficient. Document the delay here.
+        self.mcp23017.digital_write(ATT_RST_N_PIN, false).map_err(|_| Error::I2c)?;
 
         Ok(())
     }
 
     fn latch(&mut self, channel: DdsChannel) -> Result<(), Error> {
         let pin = match channel {
-            DdsChannel::Zero => ATT_LE0_PIN,
+            DdsChannel::Zero => ATT_LE1_PIN,
-            DdsChannel::One => ATT_LE1_PIN,
+            DdsChannel::One => ATT_LE0_PIN,
-            DdsChannel::Two => ATT_LE2_PIN,
+            DdsChannel::Two => ATT_LE3_PIN,
-            DdsChannel::Three => ATT_LE3_PIN,
+            DdsChannel::Three => ATT_LE2_PIN,
         };
 
-        self.mcp23017.digital_write(pin, 1).map_err(|_| Error::I2c)?;
+        self.mcp23017.digital_write(pin, true).map_err(|_| Error::I2c)?;
-        // TODO: Delay here.
+        // TODO: Measure the I2C transaction speed to the RST pin to ensure that the delay is
-        self.mcp23017.digital_write(pin, 0).map_err(|_| Error::I2c)?;
+        // sufficient. Document the delay here.
+        self.mcp23017.digital_write(pin, false).map_err(|_| Error::I2c)?;
 
         Ok(())
     }

src/pounder/rf_power.rs

@@ -7,7 +7,8 @@ pub trait PowerMeasurementInterface {
     fn measure_power(&mut self, channel: InputChannel) -> Result<f32, Error> {
         let analog_measurement = self.sample_converter(channel)?;
 
-        // The AD8363 with VSET connected to VOUT provides an output voltage of 52mV / dB.
+        // The AD8363 with VSET connected to VOUT provides an output voltage of 51.7mV / dB at
-        Ok(analog_measurement / 0.052)
+        // 100MHz.
+        Ok(analog_measurement / 0.0517)
     }
 }