Updating after review

src/bin/dual-iir.rs

@@ -40,14 +40,7 @@ pub struct Settings {
     force_hold: bool,
     telemetry_period: u16,
     stream_target: StreamTarget,
-    signal_generator: signal_generator::BasicConfig,
+    signal_generator: [signal_generator::BasicConfig; 2],
-    output_mode: [OutputMode; 2],
-}
-
-#[derive(Copy, Clone, Debug, PartialEq, Deserialize, Miniconf)]
-pub enum OutputMode {
-    IirFilter,
-    SignalGenerator,
 }
 
 impl Default for Settings {
@@ -68,10 +61,9 @@ impl Default for Settings {
             // The default telemetry period in seconds.
             telemetry_period: 10,
 
-            signal_generator: signal_generator::BasicConfig::default(),
+            signal_generator: [signal_generator::BasicConfig::default(); 2],
 
             stream_target: StreamTarget::default(),
-            output_mode: [OutputMode::IirFilter, OutputMode::IirFilter],
         }
     }
 }
@@ -85,7 +77,7 @@ const APP: () = {
         dacs: (Dac0Output, Dac1Output),
         network: NetworkUsers<Settings, Telemetry>,
         generator: BlockGenerator,
-        signal_generator: SignalGenerator,
+        signal_generator: [SignalGenerator; 2],
 
         settings: Settings,
         telemetry: TelemetryBuffer,
@@ -137,7 +129,10 @@ const APP: () = {
             digital_inputs: stabilizer.digital_inputs,
             telemetry: TelemetryBuffer::default(),
             settings,
-            signal_generator: SignalGenerator::new(settings.signal_generator),
+            signal_generator: [
+                SignalGenerator::new(settings.signal_generator[0]),
+                SignalGenerator::new(settings.signal_generator[1]),
+            ],
         }
     }
 
@@ -189,50 +184,29 @@ const APP: () = {
             fence(Ordering::SeqCst);
 
             for channel in 0..adc_samples.len() {
-                match settings.output_mode[channel] {
+                adc_samples[channel]
-                    OutputMode::IirFilter => {
+                    .iter()
-                        adc_samples[channel]
+                    .zip(dac_samples[channel].iter_mut())
+                    .zip(&mut signal_generator[channel])
+                    .map(|((ai, di), signal)| {
+                        let x = f32::from(*ai as i16);
+                        let y = settings.iir_ch[channel]
                             .iter()
-                            .zip(dac_samples[channel].iter_mut())
+                            .zip(iir_state[channel].iter_mut())
-                            .map(|(ai, di)| {
+                            .fold(x, |yi, (ch, state)| {
-                                let x = f32::from(*ai as i16);
+                                ch.update(state, yi, hold)
-                                let y = settings.iir_ch[channel]
+                            });
-                                    .iter()
-                                    .zip(iir_state[channel].iter_mut())
-                                    .fold(x, |yi, (ch, state)| {
-                                        ch.update(state, yi, hold)
-                                    });
-                                // Note(unsafe): The filter limits must ensure that the value is in range.
-                                // The truncation introduces 1/2 LSB distortion.
-                                let y: i16 = unsafe { y.to_int_unchecked() };
-                                // Convert to DAC code
-                                *di = DacCode::from(y).0;
-                            })
-                            .last();
-                    }
-                    OutputMode::SignalGenerator => {
-                        // Do not generate the samples twice, or we may mess up phasing of the
-                        // signal generator. Instead, copy the previously-generated signal.
-                        // TODO: Is there a nicer way we can handle this edge case?
-                        if (channel == 1)
-                            && settings.output_mode[0]
-                                == OutputMode::SignalGenerator
-                        {
-                            *dac_samples[1] = *dac_samples[0];
-                        } else {
-                            signal_generator
-                                .generate(&mut dac_samples[channel][..]);
-                        }
-                    }
-                }
-            }
 
-            if !settings
+                        // Note(unsafe): The filter limits must ensure that the value is in range.
-                .output_mode
+                        // The truncation introduces 1/2 LSB distortion.
-                .iter()
+                        let y: i16 = unsafe { y.to_int_unchecked() };
-                .any(|&mode| mode == OutputMode::SignalGenerator)
+
-            {
+                        let y = y.saturating_add(signal);
-                signal_generator.skip(adc_samples[0].len() as u32);
+
+                        // Convert to DAC code
+                        *di = DacCode::from(y).0;
+                    })
+                    .last();
             }
 
             // Stream the data.
@@ -273,9 +247,10 @@ const APP: () = {
         c.resources.afes.0.set_gain(settings.afe[0]);
         c.resources.afes.1.set_gain(settings.afe[1]);
 
-        // Update the signal generator
+        // Update the signal generators
         c.resources.signal_generator.lock(|generator| {
-            generator.update_waveform(settings.signal_generator)
+            generator[0].update_waveform(settings.signal_generator[0]);
+            generator[1].update_waveform(settings.signal_generator[1]);
         });
 
         let target = settings.stream_target.into();

src/bin/lockin.rs

@@ -159,16 +159,16 @@ const APP: () = {
             signal_generator: signal_generator::SignalGenerator::new(
                 signal_generator::Config {
                     // Same frequency as batch size.
-                    // TODO: Is this off-by-one?
+                    frequency: ((u32::MAX as u64 + 1u64)
-                    frequency_tuning_word: u32::MAX
+                        / design_parameters::SAMPLE_BUFFER_SIZE as u64)
-                        / design_parameters::SAMPLE_BUFFER_SIZE as u32,
+                        as u32,
 
                     // Equal symmetry
                     phase_symmetry: 0,
 
                     // 1V Amplitude
                     amplitude: ((1.0 / 10.24) * i16::MAX as f32) as i16,
-                    signal: signal_generator::Signal::Triangle,
+                    signal: signal_generator::Signal::Cosine,
                 },
             ),
 
@@ -271,7 +271,9 @@ const APP: () = {
                         // Note: Because the signal generator has a period equal to one batch size,
                         // it's okay to only update it when outputting the modulation waveform, as
                         // it will perfectly wrap back to zero phase for each batch.
-                        Conf::Modulation => signal_generator.next() as i32,
+                        Conf::Modulation => {
+                            signal_generator.next().unwrap() as i32
+                        }
                     };
 
                     *sample = DacCode::from(value as i16).0;

src/hardware/dac.rs

@@ -73,14 +73,37 @@ static mut DAC_BUF: [[SampleBuffer; 2]; 2] = [[[0; SAMPLE_BUFFER_SIZE]; 2]; 2];
 #[derive(Copy, Clone)]
 pub struct DacCode(pub u16);
 
-#[allow(clippy::from_over_into)]
+impl From<f32> for DacCode {
-impl Into<f32> for DacCode {
+    fn from(voltage: f32) -> DacCode {
-    fn into(self) -> f32 {
+        // The DAC output range in bipolar mode (including the external output op-amp) is +/- 4.096
+        // V with 16-bit resolution. The anti-aliasing filter has an additional gain of 2.5.
+        let dac_range = 4.096 * 2.5;
+
+        let voltage = if voltage > dac_range {
+            dac_range
+        } else if voltage < -1. * dac_range {
+            -1. * dac_range
+        } else {
+            voltage
+        };
+
+        DacCode::from((voltage / dac_range * i16::MAX as f32) as i16)
+    }
+}
+
+impl From<DacCode> for f32 {
+    fn from(code: DacCode) -> f32 {
         // The DAC output range in bipolar mode (including the external output op-amp) is +/- 4.096
         // V with 16-bit resolution. The anti-aliasing filter has an additional gain of 2.5.
         let dac_volts_per_lsb = 4.096 * 2.5 / (1u16 << 15) as f32;
 
-        (self.0 as i16).wrapping_add(i16::MIN) as f32 * dac_volts_per_lsb
+        (code.0 as i16).wrapping_add(i16::MIN) as f32 * dac_volts_per_lsb
+    }
+}
+
+impl From<DacCode> for i16 {
+    fn from(code: DacCode) -> i16 {
+        (code.0 as i16).wrapping_sub(i16::MIN)
     }
 }
 

src/hardware/signal_generator.rs

@@ -4,7 +4,7 @@ use serde::Deserialize;
 
 #[derive(Copy, Clone, Debug, Deserialize, Miniconf)]
 pub enum Signal {
-    Sine,
+    Cosine,
     Square,
     Triangle,
 }
@@ -12,7 +12,7 @@ pub enum Signal {
 #[derive(Copy, Clone, Debug, Miniconf, Deserialize)]
 pub struct BasicConfig {
     pub frequency: f32,
-    pub symmetry: f32,
+    pub asymmetry: f32,
     pub signal: Signal,
     pub amplitude: f32,
 }
@@ -21,40 +21,42 @@ impl Default for BasicConfig {
     fn default() -> Self {
         Self {
             frequency: 1.0e3,
-            symmetry: 0.5,
+            asymmetry: 0.0,
-            signal: Signal::Sine,
+            signal: Signal::Cosine,
-            amplitude: 1.0,
+            amplitude: 0.0,
         }
     }
 }
 
-impl Into<Config> for BasicConfig {
+impl From<BasicConfig> for Config {
-    fn into(self) -> Config {
+    fn from(config: BasicConfig) -> Config {
-        // Calculate the number of output codes in the signal period.
+        // Calculate the frequency tuning word
-        let period =
+        let frequency: u32 =
-            (100.0_e6 / (self.frequency * ADC_SAMPLE_TICKS as f32)) as u32;
+            (config.frequency * ADC_SAMPLE_TICKS as f32 / 100.0_e6
+                * (u32::MAX as u64 + 1u64) as f32) as u32;
 
         // Clamp amplitude and symmetry.
-        let amplitude = if self.amplitude > 10.24 {
+        let amplitude = if config.amplitude > 10.24 {
             10.24
+        } else if config.amplitude < 0.0 {
+            0.0
         } else {
-            self.amplitude
+            config.amplitude
         };
 
-        let symmetry = if self.symmetry < 0.0 {
+        let asymmetry = if config.asymmetry < -1.0 {
-            0.0
+            -1.0
-        } else if self.symmetry > 1.0 {
+        } else if config.asymmetry > 1.0 {
             1.0
         } else {
-            self.symmetry
+            config.asymmetry
         };
 
         Config {
-            signal: self.signal,
+            signal: config.signal,
-            amplitude: ((amplitude / 10.24) * i16::MAX as f32) as i16,
+            amplitude: DacCode::from(amplitude).into(),
-            phase_symmetry: (2.0 * (symmetry - 0.5) * i32::MAX as f32) as i32,
+            phase_symmetry: (asymmetry * i32::MAX as f32) as i32,
-            frequency_tuning_word: ((u32::MAX as u64 + 1u64) / period as u64)
+            frequency,
-                as u32,
         }
     }
 }
@@ -72,7 +74,7 @@ pub struct Config {
     pub phase_symmetry: i32,
 
     // The frequency tuning word of the signal. Phase is incremented by this amount
-    pub frequency_tuning_word: u32,
+    pub frequency: u32,
 }
 
 #[derive(Debug)]
@@ -108,16 +110,6 @@ impl SignalGenerator {
         }
     }
 
-    /// Generate a sequence of new values.
-    ///
-    /// # Args
-    /// * `samples` - The location to store generated values into.
-    pub fn generate(&mut self, samples: &mut [u16]) {
-        for sample in samples.iter_mut() {
-            *sample = DacCode::from(self.next()).0;
-        }
-    }
-
     // Increment the phase of the signal.
     //
     // # Note
@@ -128,13 +120,13 @@ impl SignalGenerator {
     fn increment(&mut self) -> i32 {
         let (phase, overflow) = self
             .phase_accumulator
-            .overflowing_add(self.config.frequency_tuning_word);
+            .overflowing_add(self.config.frequency);
 
         self.phase_accumulator = phase;
 
         // Special case: If the FTW is specified as zero, we would otherwise never update the
         // settings. Perform a check here for this corner case.
-        if overflow || self.config.frequency_tuning_word == 0 {
+        if overflow || self.config.frequency == 0 {
             if let Some(config) = self.pending_config.take() {
                 self.config = config;
                 self.phase_accumulator = 0;
@@ -144,19 +136,24 @@ impl SignalGenerator {
         self.phase_accumulator as i32
     }
 
-    /// Skip `count` elements of the generator
+    /// Update waveform generation settings.
-    pub fn skip(&mut self, count: u32) {
+    ///
-        for _ in 0..count {
+    /// # Note
-            self.increment();
+    /// Changes will not take effect until the current waveform period elapses.
-        }
+    pub fn update_waveform(&mut self, new_config: impl Into<Config>) {
+        self.pending_config = Some(new_config.into());
     }
+}
+
+impl core::iter::Iterator for SignalGenerator {
+    type Item = i16;
 
     /// Get the next value in the generator sequence.
-    pub fn next(&mut self) -> i16 {
+    fn next(&mut self) -> Option<i16> {
         let phase = self.increment();
 
         let amplitude = match self.config.signal {
-            Signal::Sine => (dsp::cossin(phase).1 >> 16) as i16,
+            Signal::Cosine => (dsp::cossin(phase).0 >> 16) as i16,
             Signal::Square => {
                 if phase < self.config.phase_symmetry {
                     i16::MAX
@@ -206,14 +203,6 @@ impl SignalGenerator {
         let result = amplitude as i32 * self.config.amplitude as i32;
 
         // Note: We downshift by 15-bits to preserve only one of the sign bits.
-        (result >> 15) as i16
+        Some((result >> 15) as i16)
-    }
-
-    /// Update waveform generation settings.
-    ///
-    /// # Note
-    /// Changes will not take effect until the current waveform period elapses.
-    pub fn update_waveform(&mut self, new_config: impl Into<Config>) {
-        self.pending_config = Some(new_config.into());
     }
 }