diff --git a/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme b/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme index 47f9c7286..81296a36f 100644 --- a/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme +++ b/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme @@ -70,7 +70,9 @@ buildConfiguration = "Debug" selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB" selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB" + enableAddressSanitizer = "YES" enableASanStackUseAfterReturn = "YES" + enableUBSanitizer = "YES" disableMainThreadChecker = "YES" launchStyle = "0" useCustomWorkingDirectory = "NO" diff --git a/OSBindings/Mac/Clock SignalTests/Bridges/DigitalPhaseLockedLoopBridge.h b/OSBindings/Mac/Clock SignalTests/Bridges/DigitalPhaseLockedLoopBridge.h index d9e841325..47ca89172 100644 --- a/OSBindings/Mac/Clock SignalTests/Bridges/DigitalPhaseLockedLoopBridge.h +++ b/OSBindings/Mac/Clock SignalTests/Bridges/DigitalPhaseLockedLoopBridge.h @@ -10,7 +10,7 @@ @interface DigitalPhaseLockedLoopBridge : NSObject -- (instancetype)initWithClocksPerBit:(NSUInteger)clocksPerBit historyLength:(NSUInteger)historyLength; +- (instancetype)initWithClocksPerBit:(NSUInteger)clocksPerBit; - (void)runForCycles:(NSUInteger)cycles; - (void)addPulse; diff --git a/OSBindings/Mac/Clock SignalTests/Bridges/DigitalPhaseLockedLoopBridge.mm b/OSBindings/Mac/Clock SignalTests/Bridges/DigitalPhaseLockedLoopBridge.mm index d04849f97..683bdca05 100644 --- a/OSBindings/Mac/Clock SignalTests/Bridges/DigitalPhaseLockedLoopBridge.mm +++ b/OSBindings/Mac/Clock SignalTests/Bridges/DigitalPhaseLockedLoopBridge.mm @@ -15,7 +15,7 @@ - (void)pushBit:(int)value; @end -class DigitalPhaseLockedLoopDelegate: public Storage::DigitalPhaseLockedLoop::Delegate { +class DigitalPhaseLockedLoopDelegate { public: __weak DigitalPhaseLockedLoopBridge *bridge; @@ -25,14 +25,14 @@ class DigitalPhaseLockedLoopDelegate: public Storage::DigitalPhaseLockedLoop::De }; @implementation DigitalPhaseLockedLoopBridge { - std::unique_ptr _digitalPhaseLockedLoop; + std::unique_ptr> _digitalPhaseLockedLoop; DigitalPhaseLockedLoopDelegate _delegate; } -- (instancetype)initWithClocksPerBit:(NSUInteger)clocksPerBit historyLength:(NSUInteger)historyLength { +- (instancetype)initWithClocksPerBit:(NSUInteger)clocksPerBit { self = [super init]; if(self) { - _digitalPhaseLockedLoop = std::make_unique((unsigned int)clocksPerBit, (unsigned int)historyLength); + _digitalPhaseLockedLoop = std::make_unique>((unsigned int)clocksPerBit); _delegate.bridge = self; _digitalPhaseLockedLoop->set_delegate(&_delegate); } diff --git a/OSBindings/Mac/Clock SignalTests/DPLLTests.swift b/OSBindings/Mac/Clock SignalTests/DPLLTests.swift index fde82b5f5..a8406f4db 100644 --- a/OSBindings/Mac/Clock SignalTests/DPLLTests.swift +++ b/OSBindings/Mac/Clock SignalTests/DPLLTests.swift @@ -26,22 +26,22 @@ class DPLLTests: XCTestCase { } func testPerfectInput() { - let pll = DigitalPhaseLockedLoopBridge(clocksPerBit: 100, historyLength: 3) + let pll = DigitalPhaseLockedLoopBridge(clocksPerBit: 100) testRegularNibblesOnPLL(pll!, bitLength: 100) } func testFastButRegular() { - let pll = DigitalPhaseLockedLoopBridge(clocksPerBit: 100, historyLength: 3) + let pll = DigitalPhaseLockedLoopBridge(clocksPerBit: 100) testRegularNibblesOnPLL(pll!, bitLength: 90) } func testSlowButRegular() { - let pll = DigitalPhaseLockedLoopBridge(clocksPerBit: 100, historyLength: 3) + let pll = DigitalPhaseLockedLoopBridge(clocksPerBit: 100) testRegularNibblesOnPLL(pll!, bitLength: 110) } func testTwentyPercentSinePattern() { - let pll = DigitalPhaseLockedLoopBridge(clocksPerBit: 100, historyLength: 3) + let pll = DigitalPhaseLockedLoopBridge(clocksPerBit: 100) var angle = 0.0 // clock in two 1s, a 0, and a 1, 200 times over diff --git a/Storage/Disk/Controller/DiskController.cpp b/Storage/Disk/Controller/DiskController.cpp index 2069795b3..8146a23be 100644 --- a/Storage/Disk/Controller/DiskController.cpp +++ b/Storage/Disk/Controller/DiskController.cpp @@ -15,10 +15,10 @@ using namespace Storage::Disk; Controller::Controller(Cycles clock_rate) : clock_rate_multiplier_(128000000 / clock_rate.as_integral()), clock_rate_(clock_rate.as_integral() * clock_rate_multiplier_), + pll_(100), empty_drive_(new Drive(int(clock_rate.as_integral()), 1, 1)) { - // seed this class with a PLL, any PLL, so that it's safe to assume non-nullptr later - Time one(1); - set_expected_bit_length(one); + pll_.set_delegate(this); + set_expected_bit_length(Time(1)); set_drive(empty_drive_); } @@ -42,13 +42,13 @@ Drive &Controller::get_drive() { void Controller::process_event(const Drive::Event &event) { switch(event.type) { - case Track::Event::FluxTransition: pll_->add_pulse(); break; + case Track::Event::FluxTransition: pll_.add_pulse(); break; case Track::Event::IndexHole: process_index_hole(); break; } } void Controller::advance(const Cycles cycles) { - if(is_reading_) pll_->run_for(Cycles(cycles.as_integral() * clock_rate_multiplier_)); + if(is_reading_) pll_.run_for(Cycles(cycles.as_integral() * clock_rate_multiplier_)); } void Controller::process_write_completed() { @@ -66,9 +66,8 @@ void Controller::set_expected_bit_length(Time bit_length) { // this conversion doesn't need to be exact because there's a lot of variation to be taken // account of in rotation speed, air turbulence, etc, so a direct conversion will do - int clocks_per_bit = cycles_per_bit.get(); - pll_ = std::make_unique(clocks_per_bit, 3); - pll_->set_delegate(this); + const int clocks_per_bit = cycles_per_bit.get(); + pll_.set_clocks_per_bit(clocks_per_bit); } void Controller::digital_phase_locked_loop_output_bit(int value) { diff --git a/Storage/Disk/Controller/DiskController.hpp b/Storage/Disk/Controller/DiskController.hpp index c177f4d12..fa876ea6e 100644 --- a/Storage/Disk/Controller/DiskController.hpp +++ b/Storage/Disk/Controller/DiskController.hpp @@ -29,7 +29,6 @@ namespace Disk { TODO: communication of head size and permissible stepping extents, appropriate simulation of gain. */ class Controller: - public DigitalPhaseLockedLoop::Delegate, public Drive::EventDelegate, public ClockingHint::Source, public ClockingHint::Observer { @@ -111,7 +110,9 @@ class Controller: bool is_reading_ = true; - std::shared_ptr pll_; + DigitalPhaseLockedLoop pll_; + friend DigitalPhaseLockedLoop; + std::shared_ptr drive_; std::shared_ptr empty_drive_; @@ -124,7 +125,7 @@ class Controller: void advance(const Cycles cycles) override ; // to satisfy DigitalPhaseLockedLoop::Delegate - void digital_phase_locked_loop_output_bit(int value) override; + void digital_phase_locked_loop_output_bit(int value); }; } diff --git a/Storage/Disk/DPLL/DigitalPhaseLockedLoop.cpp b/Storage/Disk/DPLL/DigitalPhaseLockedLoop.cpp index fe6409b01..2e4ee02b7 100644 --- a/Storage/Disk/DPLL/DigitalPhaseLockedLoop.cpp +++ b/Storage/Disk/DPLL/DigitalPhaseLockedLoop.cpp @@ -12,58 +12,3 @@ using namespace Storage; -DigitalPhaseLockedLoop::DigitalPhaseLockedLoop(int clocks_per_bit, std::size_t length_of_history) : - offset_history_(length_of_history, 0), - window_length_(clocks_per_bit), - clocks_per_bit_(clocks_per_bit) {} - -void DigitalPhaseLockedLoop::run_for(const Cycles cycles) { - offset_ += cycles.as_integral(); - phase_ += cycles.as_integral(); - if(phase_ >= window_length_) { - auto windows_crossed = phase_ / window_length_; - - // check whether this triggers any 0s, if anybody cares - if(delegate_) { - if(window_was_filled_) --windows_crossed; - for(int c = 0; c < windows_crossed; c++) - delegate_->digital_phase_locked_loop_output_bit(0); - } - - window_was_filled_ = false; - phase_ %= window_length_; - } -} - -void DigitalPhaseLockedLoop::add_pulse() { - if(!window_was_filled_) { - if(delegate_) delegate_->digital_phase_locked_loop_output_bit(1); - window_was_filled_ = true; - post_phase_offset(phase_, offset_); - offset_ = 0; - } -} - -void DigitalPhaseLockedLoop::post_phase_offset(Cycles::IntType new_phase, Cycles::IntType new_offset) { - offset_history_[offset_history_pointer_] = new_offset; - offset_history_pointer_ = (offset_history_pointer_ + 1) % offset_history_.size(); - - // use an unweighted average of the stored offsets to compute current window size, - // bucketing them by rounding to the nearest multiple of the base clocks per bit - Cycles::IntType total_spacing = 0; - Cycles::IntType total_divisor = 0; - for(auto offset : offset_history_) { - auto multiple = (offset + (clocks_per_bit_ >> 1)) / clocks_per_bit_; - if(!multiple) continue; - total_divisor += multiple; - total_spacing += offset; - } - if(total_divisor) { - window_length_ = total_spacing / total_divisor; - } - - auto error = new_phase - (window_length_ >> 1); - - // use a simple spring mechanism as a lowpass filter for phase - phase_ -= (error + 1) >> 1; -} diff --git a/Storage/Disk/DPLL/DigitalPhaseLockedLoop.hpp b/Storage/Disk/DPLL/DigitalPhaseLockedLoop.hpp index 6fd2764c7..41c25ed07 100644 --- a/Storage/Disk/DPLL/DigitalPhaseLockedLoop.hpp +++ b/Storage/Disk/DPLL/DigitalPhaseLockedLoop.hpp @@ -9,6 +9,8 @@ #ifndef DigitalPhaseLockedLoop_hpp #define DigitalPhaseLockedLoop_hpp +#include +#include #include #include @@ -16,54 +18,125 @@ namespace Storage { -class DigitalPhaseLockedLoop { +/*! + Template parameters: + + @c length_of_history The number of historic pulses to consider in locking to phase. +*/ +template class DigitalPhaseLockedLoop { public: /*! Instantiates a @c DigitalPhaseLockedLoop. @param clocks_per_bit The expected number of cycles between each bit of input. - @param length_of_history The number of historic pulses to consider in locking to phase. */ - DigitalPhaseLockedLoop(int clocks_per_bit, std::size_t length_of_history); + DigitalPhaseLockedLoop(int clocks_per_bit) : + window_length_(clocks_per_bit), clocks_per_bit_(clocks_per_bit) {} + + /*! + Changes the expected window length. + */ + void set_clocks_per_bit(int clocks_per_bit) { + window_length_ = clocks_per_bit_ = clocks_per_bit; + } /*! Runs the loop, impliedly posting no pulses during that period. @c number_of_cycles The time to run the loop for. */ - void run_for(const Cycles cycles); + void run_for(const Cycles cycles) { + offset_ += cycles.as_integral(); + phase_ += cycles.as_integral(); + if(phase_ >= window_length_) { + auto windows_crossed = phase_ / window_length_; + + // check whether this triggers any 0s, if anybody cares + if(delegate_) { + if(window_was_filled_) --windows_crossed; + for(int c = 0; c < windows_crossed; c++) + delegate_->digital_phase_locked_loop_output_bit(0); + } + + window_was_filled_ = false; + phase_ %= window_length_; + } + } /*! Announces a pulse at the current time. */ - void add_pulse(); + void add_pulse() { + if(!window_was_filled_) { + if(delegate_) delegate_->digital_phase_locked_loop_output_bit(1); + window_was_filled_ = true; + post_phase_offset(phase_, offset_); + offset_ = 0; + } + } /*! A receiver for PCM output data; called upon every recognised bit. */ - class Delegate { - public: - virtual void digital_phase_locked_loop_output_bit(int value) = 0; - }; - void set_delegate(Delegate *delegate) { + void set_delegate(BitHandler *delegate) { delegate_ = delegate; } private: - Delegate *delegate_ = nullptr; + BitHandler *delegate_ = nullptr; - void post_phase_offset(Cycles::IntType phase, Cycles::IntType offset); + void post_phase_offset(Cycles::IntType new_phase, Cycles::IntType new_offset) { + // Erase the effect of whatever is currently in this slot. + total_divisor_ -= offset_history_[offset_history_pointer_].divisor; + total_spacing_ -= offset_history_[offset_history_pointer_].spacing; - std::vector offset_history_; + // Fill in the new fields. + const auto multiple = (new_offset + (clocks_per_bit_ >> 1)) / clocks_per_bit_; + offset_history_[offset_history_pointer_].divisor = multiple; + offset_history_[offset_history_pointer_].spacing = new_offset; + + // Add in the new values; + total_divisor_ += offset_history_[offset_history_pointer_].divisor; + total_spacing_ += offset_history_[offset_history_pointer_].spacing; + + // Advance the write slot. + offset_history_pointer_ = (offset_history_pointer_ + 1) % offset_history_.size(); + + // In net: use an unweighted average of the stored offsets to compute current window size, + // bucketing them by rounding to the nearest multiple of the base clocks per bit + window_length_ = total_spacing_ / total_divisor_; +#ifndef NDEBUG + bool are_all_filled = true; + for(auto offset: offset_history_) { + if(offset.spacing == 1) { + are_all_filled = false; + break; + } + } + assert(!are_all_filled || (window_length_ >= ((clocks_per_bit_ * 9) / 10) && window_length_ <= ((clocks_per_bit_ * 11) / 10))); +#endif + + // Also apply a difference to phase, use a simple spring mechanism as a lowpass filter. + const auto error = new_phase - (window_length_ >> 1); + phase_ -= (error + 1) >> 1; + } + + struct LoggedOffset { + Cycles::IntType divisor = 1, spacing = 1; + }; + std::array offset_history_; std::size_t offset_history_pointer_ = 0; - Cycles::IntType offset_ = 0; + + Cycles::IntType total_spacing_ = length_of_history; + Cycles::IntType total_divisor_ = length_of_history; Cycles::IntType phase_ = 0; Cycles::IntType window_length_ = 0; + + Cycles::IntType offset_ = 0; bool window_was_filled_ = false; int clocks_per_bit_ = 0; - int tolerance_ = 0; }; } diff --git a/Storage/Disk/Track/TrackSerialiser.cpp b/Storage/Disk/Track/TrackSerialiser.cpp index 61b6d845c..f8590fd6c 100644 --- a/Storage/Disk/Track/TrackSerialiser.cpp +++ b/Storage/Disk/Track/TrackSerialiser.cpp @@ -14,18 +14,18 @@ // just return a copy of that segment. Storage::Disk::PCMSegment Storage::Disk::track_serialisation(const Track &track, Time length_of_a_bit) { unsigned int history_size = 16; - DigitalPhaseLockedLoop pll(100, history_size); std::unique_ptr track_copy(track.clone()); // ResultAccumulator exists to append whatever comes out of the PLL to // its PCMSegment. - struct ResultAccumulator: public DigitalPhaseLockedLoop::Delegate { + struct ResultAccumulator { PCMSegment result; void digital_phase_locked_loop_output_bit(int value) { result.data.push_back(!!value); } } result_accumulator; result_accumulator.result.length_of_a_bit = length_of_a_bit; + DigitalPhaseLockedLoop pll(100); // Obtain a length multiplier which is 100 times the reciprocal // of the expected bit length. So a perfect bit length from diff --git a/Storage/Tape/Parsers/Acorn.cpp b/Storage/Tape/Parsers/Acorn.cpp index 7101a3d05..1997f4d5a 100644 --- a/Storage/Tape/Parsers/Acorn.cpp +++ b/Storage/Tape/Parsers/Acorn.cpp @@ -66,7 +66,7 @@ void Parser::process_pulse(const Storage::Tape::Tape::Pulse &pulse) { Shifter::Shifter() : - pll_(PLLClockRate / 4800, 15), + pll_(PLLClockRate / 4800), was_high_(false), input_pattern_(0), input_bit_counter_(0), diff --git a/Storage/Tape/Parsers/Acorn.hpp b/Storage/Tape/Parsers/Acorn.hpp index 937873b64..3dd9c32d3 100644 --- a/Storage/Tape/Parsers/Acorn.hpp +++ b/Storage/Tape/Parsers/Acorn.hpp @@ -17,7 +17,7 @@ namespace Storage { namespace Tape { namespace Acorn { -class Shifter: public Storage::DigitalPhaseLockedLoop::Delegate { +class Shifter { public: Shifter(); @@ -34,7 +34,7 @@ class Shifter: public Storage::DigitalPhaseLockedLoop::Delegate { void digital_phase_locked_loop_output_bit(int value); private: - Storage::DigitalPhaseLockedLoop pll_; + Storage::DigitalPhaseLockedLoop pll_; bool was_high_; unsigned int input_pattern_;