Control Pounder through MQTT #610

occheung · 2022-08-30T10:51:02Z

Description

This PR implements control of devices on Pounder through MQTT.

These parameters are configurable through MQTT (topic prefix: <prefix>/settings/pdh/ch/<n>).

pub struct PDHChannel {
    pub osc_frequency: f32,     // Frequency of the IN/OUT DDS pair
    pub out_phase: f32,         // Phase offset of the OUT DDS channel
    pub in_phase_offset: f32,   // Phase offset of the IN DDS channel relative to the OUT DDS channel
    pub amplitude: f32,         // Amplitude of the OUT DDS channel
    pub in_attenuation: f32,    // Attenuation of the signal injected to INx connector
    pub out_attenuation: f32,   // Attenuation of the OUT DDS channel
    pub enabled: bool,          // Amplitude of the IN DDS channel (0.0 or 1.0)
}

Temperature reading from Pounder (LM75) & input powers (after attenuators) are reported in telemetry.

I also tried reporting power level through the LED, but I got issues like AttRstN being stuck at low voltage. I have another branch in my fork that has working LED using the mcp23017 library.

It is just to avoid index confusion. The datasheet starts the index from 0 as well.

occheung · 2022-09-01T02:19:54Z

Added the same Pounder telemetry to lockin variant.

@jordens What are the plans of Pounder for lockin?

src/bin/dual-iir.rs

ad9959/src/lib.rs

src/bin/dual-iir.rs

ad9959/src/lib.rs

src/bin/dual-iir.rs

src/net/telemetry.rs

src/bin/dual-iir.rs

jordens · 2022-09-01T08:11:54Z

src/hardware/pounder/mod.rs

+    pub ch: [PDHChannel; 2],
+}
+
+impl Default for PDHLockGeneratorConfig {


jordens · 2022-09-01T08:13:16Z

There are no plans for Pounder and lockin.

- Packed pounder telemetries into `PounderTelemetry`. - Moved CPU temperature & Pounder telemetries into the telemetry buffer.

Just to make sense of variable accessing.

ryan-summers

This is looking much better! Most of my comments are related to documentation and clarifying magic numbers in various files.

There's quite a few things Robert pointed out that still need clean up as well

ryan-summers · 2023-01-17T09:21:52Z

ad9959/src/lib.rs

        // TODO: Update / disable any enabled channels?
        let mut fr1: [u8; 3] = [0, 0, 0];
        self.read(Register::FR1, &mut fr1)?;
        fr1[0].set_bits(2..=6, multiplier);

-        let vco_range = frequency > 255e6;
+        let vco_range = frequency > 200e6;


Please move the 200e6 to a const variable with a name at the top of the file. Also, can you please document where this limit is coming from? It's not clear to me (nor can I remember why the original 255e6 was selected)

Documented. Its selection is somewhat arbitrary.

I talk about this below, but we should just use the min of the high VCO gain config or max of the low VCO gain config constants instead of this arbitrary value.

ad9959/src/lib.rs

src/hardware/pounder/mod.rs

src/hardware/setup.rs

ryan-summers · 2023-01-17T09:48:23Z

src/hardware/setup.rs

+            if self.pounder.get_attenuation(pounder_channel).unwrap()
+                != channel_config.attenuation
+            {
+                self.pounder
+                    .set_attenuation(
+                        pounder_channel,
+                        channel_config.attenuation,
+                    )
+                    .unwrap();
+            }


I don't see any downside to unconditionally writing the attenuation (even if it's redundant). Is there any transient that occurs when we latch the attenuation? If not, I'd simplify this and just always write attenuation updates.

Then should we just have a function that sets attenuation of all channels in 1 go? We are currently repeating the SPI write (read + update) 4 times when only 1 write is needed.
Then we don't need to think about preserving other channel's attenuation with a read sequence, since we are writing unconditionally.

It really just comes down to how often we're updating attenuation and when we do it. If we need the efficiency, then by all means write all of them in one go, but if it doesn't matter, lets leave what we have in place and open an issue for the future

src/net/telemetry.rs

Co-authored-by: Ryan Summers <summers.ryan.m@gmail.com>

ryan-summers

Looking better, but there's still a few things to clean up. Thanks for working on this. I think there's also some outstanding conversations form @jordens and I earlier as well that still need to be addressed.

ryan-summers · 2023-01-30T09:20:55Z

ad9959/src/lib.rs

+/// An arbitrary frequency that sits between the 160 MHz to 255 MHz zone, where system clock
+/// frequency is not supported by both high frequency and low frequency VCO settings.
+const VCO_MIN_FREQUENCY: f32 = 200e6;


We don't allow configuration in the 160-255MHz range based on the clocking checks below. We should remove this constant and just use either the min (high gain) or max (low gain) as a switch for the VCO gain bit.

ryan-summers · 2023-01-30T09:23:52Z

ad9959/src/lib.rs

+/// The maximum system clock frequency with low frequency range VCO.
+const MAX_OUTPUT_FREQUENCY: f32 = 160e6;
+/// The minimum system clock frequency with low frequency range VCO.
+const MIN_OUTPUT_FREQUENCY: f32 = 100e6;


Suggested change

/// The maximum system clock frequency with low frequency range VCO.

const MAX_OUTPUT_FREQUENCY: f32 = 160e6;

/// The minimum system clock frequency with low frequency range VCO.

const MIN_OUTPUT_FREQUENCY: f32 = 100e6;

/// The system clock frequency range with low gain configured for the internal VCO.

const LOW_GAIN_VCO_RANGE: Range<f32> = core::ops::Range { start: 100e6, end: 160e6 };

ryan-summers · 2023-01-30T09:24:20Z

ad9959/src/lib.rs

+/// The maximum system clock frequency with high frequency range VCO.
+const MAX_VCO_OUTPUT_FREQUENCY: f32 = 500e6;
+/// The minimum system clock frequency with high frequency range VCO.
+const MIN_VCO_OUTPUT_FREQUENCY: f32 = 255e6;


Suggested change

/// The maximum system clock frequency with high frequency range VCO.

const MAX_VCO_OUTPUT_FREQUENCY: f32 = 500e6;

/// The minimum system clock frequency with high frequency range VCO.

const MIN_VCO_OUTPUT_FREQUENCY: f32 = 255e6;

/// The system clock frequency range with high gain configured for the internal VCO.

const HIGH_GAIN_VCO_RANGE: Range<f32> = core::ops::Range { start: 255e6, end: 500e6 };

ryan-summers · 2023-01-30T09:26:50Z

ad9959/src/lib.rs

+    // The REFCLK frequency must be at least 1 MHz with REFCLK multiplier disabled.
+    if multiplier == 1 && reference_clock_frequency < MIN_REFCLK_FREQUENCY {
+        return Err(Error::Bounds);
+    }


Move this to the top of the function and remove the check for multiplier == 1. In reality, 1MHz is the absolute lowest allowed REFCLK, but the limit might be higher. We never should allow a lower refclk.

ryan-summers · 2023-01-30T09:29:52Z

ad9959/src/lib.rs

+/// Convert amplitude into amplitude control register values.
+///
+/// Arguments:
+/// * `amplitude` - The amplitude scaling factor of a DDS channel.


Suggested change

/// * `amplitude` - The amplitude scaling factor of a DDS channel.

/// * `amplitude` - The normalized amplitude of a DDS channel.

ryan-summers · 2023-01-30T09:33:43Z

ad9959/src/lib.rs

+    let amplitude_control: u16 = (amplitude * (1 << 10) as f32) as u16;
+
+    // Enable the amplitude multiplier for the channel if required. The amplitude control has
+    // full-scale at 0x3FF (amplitude of 1), so the multiplier should be disabled whenever
+    // full-scale is used.
+    let acr = if amplitude != 1.0 {
+        // Enable the amplitude multiplier
+        (amplitude_control & 0x3FF) | (1 << 12)
+    } else {
+        0
+    };


Suggested change

let amplitude_control: u16 = (amplitude * (1 << 10) as f32) as u16;

// Enable the amplitude multiplier for the channel if required. The amplitude control has

// full-scale at 0x3FF (amplitude of 1), so the multiplier should be disabled whenever

// full-scale is used.

let acr = if amplitude != 1.0 {

// Enable the amplitude multiplier

(amplitude_control & 0x3FF) | (1 << 12)

} else {

0

};

let acr: u32 = 0u32.set_bits(0..=9, (amplitude * (1 << 10) as f32) as u16).set_bit(12, amplitude != 1.0);

Use the bit_field API: https://docs.rs/bit_field/0.10.1/bit_field/

ryan-summers · 2023-01-30T09:47:29Z

src/bin/dual-iir.rs

@@ -199,6 +214,7 @@ mod app {
        adcs: (Adc0Input, Adc1Input),
        dacs: (Dac0Output, Dac1Output),
        iir_state: [[iir::Vec5<f32>; IIR_CASCADE_LENGTH]; 2],
+        dds_clock_state: Option<ClockConfig>,


This never actually changes in the code. Why do we store it as a Resource? It seems like this should just exist within Pounder instead.

ryan-summers · 2023-01-30T09:52:07Z

src/net/telemetry.rs

+    pub temperature: f32,
+
+    /// The detected RF power into IN channels
+    pub input_powers: [f32; 2],


Suggested change

pub input_powers: [f32; 2],

pub input_power: [f32; 2],

It's already plural because it's an array, naming it input_powers implies each channel has multiple power measurements

ryan-summers · 2023-01-30T09:52:20Z

src/net/telemetry.rs

+/// overhead.
+#[derive(Copy, Clone, Serialize)]
+pub struct PounderTelemetry {
+    /// The Pounder temperature in degree Celsius


Suggested change

/// The Pounder temperature in degree Celsius

/// The Pounder temperature in degrees Celsius

ryan-summers · 2023-01-30T09:53:00Z

src/hardware/setup.rs

+impl PounderDevices {
+    pub fn update_dds(
+        &mut self,
+        settings: PounderConfig,
+        clocking: &mut ClockConfig,
+    ) {
+        if *clocking != settings.clock {
+            match validate_clocking(
+                settings.clock.reference_clock,
+                settings.clock.multiplier,
+            ) {
+                Ok(_frequency) => {
+                    self.pounder
+                        .set_ext_clk(settings.clock.external_clock)
+                        .unwrap();
+
+                    self.dds_output
+                        .builder()
+                        .set_system_clock(
+                            settings.clock.reference_clock,
+                            settings.clock.multiplier,
+                        )
+                        .unwrap()
+                        .write();
+
+                    *clocking = settings.clock;
+                }
+                Err(err) => {
+                    log::error!("Invalid AD9959 clocking parameters: {:?}", err)
+                }
+            }
+        }
+
+        for (channel_config, pounder_channel) in settings
+            .in_channel
+            .iter()
+            .chain(settings.out_channel.iter())
+            .zip([
+                PounderChannel::In0,
+                PounderChannel::In1,
+                PounderChannel::Out0,
+                PounderChannel::Out1,
+            ])
+        {
+            match Profile::try_from((*clocking, *channel_config)) {
+                Ok(dds_profile) => {
+                    self.dds_output
+                        .builder()
+                        .update_channels_with_profile(
+                            pounder_channel.into(),
+                            dds_profile,
+                        )
+                        .write();
+
+                    if let Err(err) = self.pounder.set_attenuation(
+                        pounder_channel,
+                        channel_config.attenuation,
+                    ) {
+                        log::error!("Invalid attenuation settings: {:?}", err)
+                    }
+                }
+                Err(err) => {
+                    log::error!("Invalid AD9959 profile settings: {:?}", err)
+                }
+            }
+        }
+    }
+
+    pub fn get_telemetry(&mut self) -> PounderTelemetry {
+        PounderTelemetry {
+            temperature: self.pounder.lm75.read_temperature().unwrap(),
+            input_powers: [
+                self.pounder.measure_power(PounderChannel::In0).unwrap(),
+                self.pounder.measure_power(PounderChannel::In1).unwrap(),
+            ],
+        }
+    }
+}
+


This belongs in hardware/pounder/ somewhere, not the setup utilities.

sbourdeauducq · 2023-04-24T03:18:30Z

Closed in favor of #725

occheung added 9 commits August 30, 2022 18:05

dds profile: expose dds clock reconfig

0e2b4d1

ad9959: make channel enum start from ZERO

374187a

It is just to avoid index confusion. The datasheet starts the index from 0 as well.

update pounder::Channel to ad9959::Channel conversion

e6dac8e

pounder mqtt: add topics

35def7f

dual-iir: handle mqtt pounder topics

deab689

ad9959: enlarge serialzier buffer

96b76d2

telemetry: add pounder temperature & input powers

a4db1bb

cargo fmt

d4b1737

add pounder telemetry to lockin

4e6c16d

occheung changed the title ~~[WIP]: Control Pounder through MQTT~~ Control Pounder through MQTT Sep 1, 2022

occheung marked this pull request as ready for review September 1, 2022 02:20

fix struct path in doc

f11f0fe

ryan-summers reviewed Sep 1, 2022

View reviewed changes

src/bin/dual-iir.rs Outdated Show resolved Hide resolved

jordens requested changes Sep 1, 2022

View reviewed changes

occheung added 14 commits September 1, 2022 17:15

wrap pdh config in option

00558d6

add stricter frequency checks

ecf4d23

fix comment regarding assumption of FR1 value

9720452

move vco_range threshold

d5aaaef

only reconfig dds clock when requested

6531493

use ad9959 idea of amp-to-acr conversion for serializer

3509991

replace hex with shifts in phase-to-pow

8d8cbde

remove unnecessary wrapping shift

c5d94d7

limit dds frequency within the first Nyquist zone

077b634

drop f_foo in favor of foo_frequency

58ee3ee

update telemetry structure

f9459ae

- Packed pounder telemetries into `PounderTelemetry`. - Moved CPU temperature & Pounder telemetries into the telemetry buffer.

cargo fmt

6f69ec1

lockin: update telemetry handling

e28c396

dual-iir: reshuffle telemetry write order

86935da

Just to make sense of variable accessing.

occheung added 5 commits January 10, 2023 12:43

resolve merge conflict

36cf203

lockin: remove pounder

9e9426d

use updated miniconf interface

6b3a2e7

cargo fmt

d227164

mqtt: use miniconf primitive for defer

f0bc6da

sbourdeauducq requested a review from ryan-summers January 16, 2023 08:36

ryan-summers requested changes Jan 17, 2023

View reviewed changes

occheung and others added 19 commits January 19, 2023 20:33

settings: replace field instead

3520292

Co-authored-by: Ryan Summers <summers.ryan.m@gmail.com>

ad9959: clean up frequency checking

926fb58

ad9959: revert set_amplitude

dd15b43

ad9959: revert bit operations

3b9446f

ad9959: add limitation to set_system_clock

2751ddc

ad9959: lighter amplitude check code

d20d404

ad9959: add docs for converters

e2a32af

migrate ad9959 Profile to pounder

e952d8a

pounder: in/out_ch -> in/out_channel

2824527

dds_output: update profile fields reference

5edcad8

pounder: handle errors & log

bd8e2df

mqtt: remove pounder telem from buffer

a98fe06

settings: remove pounder telem setter

370e19c

ad9959: specify the amplitude is a scaling

3ec95fd

remove profile wrapper

cfd6961

ad9959: make doc for constants

2361a75

clocking: detach from designed params

bd69e57

Profile: comment -> docstring

76e66cb

set_system_clock: fix docstring brackets

512bb20

ryan-summers requested changes Jan 30, 2023

View reviewed changes

Spaqin mentioned this pull request Apr 17, 2023

Control Pounder through MQTT - finished #725

Closed

sbourdeauducq closed this Apr 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Control Pounder through MQTT #610

Control Pounder through MQTT #610

occheung commented Aug 30, 2022

occheung commented Sep 1, 2022

jordens Sep 1, 2022

jordens commented Sep 1, 2022 •

edited

Loading

ryan-summers left a comment

ryan-summers Jan 17, 2023

occheung Jan 20, 2023

ryan-summers Jan 30, 2023

ryan-summers Jan 17, 2023

occheung Jan 20, 2023 •

edited

Loading

ryan-summers Jan 30, 2023

ryan-summers left a comment

ryan-summers Jan 30, 2023

ryan-summers Jan 30, 2023

ryan-summers Jan 30, 2023

ryan-summers Jan 30, 2023

ryan-summers Jan 30, 2023

ryan-summers Jan 30, 2023

ryan-summers Jan 30, 2023

ryan-summers Jan 30, 2023

ryan-summers Jan 30, 2023

ryan-summers Jan 30, 2023

ryan-summers Jan 30, 2023

sbourdeauducq commented Apr 24, 2023

	/// * `amplitude` - The amplitude scaling factor of a DDS channel.
	/// * `amplitude` - The normalized amplitude of a DDS channel.

	/// The Pounder temperature in degree Celsius
	/// The Pounder temperature in degrees Celsius

Control Pounder through MQTT #610

Control Pounder through MQTT #610

Conversation

occheung commented Aug 30, 2022

Description

occheung commented Sep 1, 2022

Choose a reason for hiding this comment

jordens commented Sep 1, 2022 • edited Loading

ryan-summers left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

occheung Jan 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryan-summers left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbourdeauducq commented Apr 24, 2023

jordens commented Sep 1, 2022 •

edited

Loading

occheung Jan 20, 2023 •

edited

Loading