From 6b663894b061a401b7335ce5ad62c4ef913b5ddc Mon Sep 17 00:00:00 2001 From: stafur Date: Sun, 1 Mar 2026 15:51:28 -0500 Subject: [PATCH] Addresses comments from PR #3057 (#3058) * Addressed clock inversion and fractiona/interger states. Created ui_debug display that checks expected clock frequencies for expected 3.072 MHz sampling frequency commonly utilizaed in portapack apps. * Ran format-code.sh * Addressed comments from PR. Ran format-code.sh. Updated definition of a, b, and c in comments and introduced x, y, and z for debug display. Changed default color for P2/P3 conditional debub display for MS_DIV. Removed implied recalibration since this did not address crackling sound. --- firmware/application/apps/ui_debug.cpp | 28 +++++++++++++++++--------- firmware/application/apps/ui_debug.hpp | 12 +++++------ firmware/application/clock_manager.cpp | 4 +--- 3 files changed, 25 insertions(+), 19 deletions(-) diff --git a/firmware/application/apps/ui_debug.cpp b/firmware/application/apps/ui_debug.cpp index 54dfe83c5..6c9fc2e55 100644 --- a/firmware/application/apps/ui_debug.cpp +++ b/firmware/application/apps/ui_debug.cpp @@ -2347,34 +2347,42 @@ void Si5351MultiSynthDebugView::refresh() { } else if (p3 > 1) { text_p2.set_style(Theme::getInstance()->fg_green); text_p3.set_style(Theme::getInstance()->fg_green); + } else { + // Default/neutral style when P2/P3 don't match known patterns + text_p2.set_style(Theme::getInstance()->fg_light); + text_p3.set_style(Theme::getInstance()->fg_light); } // === Calculate Output Frequency === - // + // === Calculate Multisynth Divider === // Correct Si5351 formula: // MS_DIV = (P2+P3 × (P1 + 512)) / (128 × P3) // MS_DIV = P2/(128*P3) + P1+512/(128*P3) - // a = (P1 + 512) / 128 - // b = P2/128 - // c = P3 + // x = (P1 + 512) / 128 + // y = P2/128 + // z = P3 // f_out = f_vco / MS_DIV / R_DIV - // For integer division: b=0, c=1, so MS_DIV = a + // a = floor((P1 + 512) / 128) + // k = (P1 + 512) - 128*a + // b = b = (P2 + c*k) / 128 + // c = P3 + // In the case of integer division: b=0, c=1, so MS_DIV = a uint64_t ms_div_numerator = (uint64_t)p2 + (uint64_t)p3 * (p1 + 512); uint64_t ms_div_denominator = 128ULL * p3; - uint32_t a = (p1 + 512) / 128; - uint32_t b = p2 / 128; - uint32_t c = p3; + uint32_t x = (p1 + 512) / 128; + uint32_t y = p2 / 128; + uint32_t z = p3; // For display, show the full fractional value // MS_DIV = (a+b)/c if (p3 > 1 && p2 > 0) { - std::string div_str = "(" + to_string_dec_uint(a) + "+" + to_string_dec_uint(b) + ")/" + to_string_dec_uint(c); + std::string div_str = "(" + to_string_dec_uint(x) + "+" + to_string_dec_uint(y) + ")/" + to_string_dec_uint(z); text_div.set(div_str); } else { - std::string div_str = to_string_dec_uint(a); + std::string div_str = to_string_dec_uint(x); text_div.set(div_str); } diff --git a/firmware/application/apps/ui_debug.hpp b/firmware/application/apps/ui_debug.hpp index ca38c2ed5..8debfa625 100644 --- a/firmware/application/apps/ui_debug.hpp +++ b/firmware/application/apps/ui_debug.hpp @@ -1126,12 +1126,12 @@ class SystemDiagnosticsView : public View { #endif #ifdef PRALINE -/* Si5351MultiSynthDebugView - Debug fractional register values for MS0/MS1 */ +/* Si5351MultiSynthDebugView - Debug fractional register values for MS0 */ class Si5351MultiSynthDebugView : public View { public: Si5351MultiSynthDebugView(NavigationView& nav); void focus() override; - std::string title() const override { return "Si5351 MS Debug"; }; + std::string title() const override { return "Si5351 MS0 Debug"; }; private: NavigationView& nav_; @@ -1139,7 +1139,7 @@ class Si5351MultiSynthDebugView : public View { void force_pll_reset(); void force_fractional_mode(); - Text text_title{{0, 0, 240, 16}, "=== MS0 Fractional Debug ==="}; + Text text_title{{0, 0, 240, 16}, "===MS0 Fractional Debug==="}; // Clock Control Register 16 (CLK0) Text text_lbl_clk_ctrl{{0, 18, 100, 16}, "CLK0 Ctrl:"}; @@ -1149,7 +1149,7 @@ class Si5351MultiSynthDebugView : public View { Text text_ms_int{{102, 34, 138, 16}, "---"}; // Raw register values - Text text_lbl_raw{{0, 54, 240, 16}, "--- Raw Registers (42-49) ---"}; + Text text_lbl_raw{{0, 54, 240, 16}, "---Raw Registers (42-49)---"}; Text text_lbl_r42_43{{0, 70, 80, 16}, "R42-43:"}; Text text_r42_43{{82, 70, 158, 16}, "---"}; @@ -1161,7 +1161,7 @@ class Si5351MultiSynthDebugView : public View { Text text_r47_49{{82, 102, 158, 16}, "---"}; // Decoded values - Text text_lbl_decoded{{0, 122, 240, 16}, "--- Decoded Values ---"}; + Text text_lbl_decoded{{0, 122, 240, 16}, "---Decoded Values---"}; Text text_lbl_p1{{0, 138, 48, 16}, "P1:"}; Text text_p1{{50, 138, 190, 16}, "---"}; @@ -1176,7 +1176,7 @@ class Si5351MultiSynthDebugView : public View { Text text_rdiv{{50, 186, 190, 16}, "---"}; // Calculated frequency - Text text_lbl_calc{{0, 206, 240, 16}, "--- Calculated Output ---"}; + Text text_lbl_calc{{0, 206, 240, 16}, "---Calculated Output---"}; Text text_lbl_div{{0, 222, 80, 16}, "MS Div:"}; Text text_div{{82, 222, 158, 16}, "---"}; diff --git a/firmware/application/clock_manager.cpp b/firmware/application/clock_manager.cpp index f3c508484..fd19c725c 100644 --- a/firmware/application/clock_manager.cpp +++ b/firmware/application/clock_manager.cpp @@ -247,7 +247,7 @@ constexpr ClockControls si5351c_clock_control_common{{ constexpr ClockControls si5351a_clock_control_common{{ #ifdef PRALINE - // CLK0: MAX5864 (ADC) - 4mA, Inverted (Standard for Praline sync) + // CLK0: MAX5864 (ADC) - 4mA, Normal (Standard for Praline sync) {ClockControl::ClockCurrentDrive::_4mA, ClockControl::ClockSource::MS_Self, ClockControl::ClockInvert::Normal, ClockControl::MultiSynthSource::PLLA, ClockControl::MultiSynthMode::Fractional, ClockControl::ClockPowerDown::Power_On}, // CLK1: SCT_CLK (iCE40 FPGA) - 6mA, Inverted (Fixes 30-60Hz Drumming) {ClockControl::ClockCurrentDrive::_6mA, ClockControl::ClockSource::MS_Self, ClockControl::ClockInvert::Invert, ClockControl::MultiSynthSource::PLLA, ClockControl::MultiSynthMode::Fractional, ClockControl::ClockPowerDown::Power_On}, @@ -722,8 +722,6 @@ void ClockManager::set_sampling_frequency(const uint32_t frequency) { clock_generator.set_ms_frequency(0, afe_rate * 2, si5351_vco_f, 1); clock_generator.set_ms_frequency(1, afe_rate * 2, si5351_vco_f, 0); - radio::invalidate_spi_config(); // Triggers the MAX2831 to recalibrate - #else /* Codec clock is at sampling frequency, CPLD and SGPIO clocks are at * twice the frequency, and derived from the MS0 synth. So it's only