From f42655a0fcf422b894fea2cb6113f4dd25d91767 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 12 Jan 2020 17:25:21 -0500 Subject: [PATCH 1/4] Promote DigitalPhaseLockedLoop to a template, simplify to O(1) add_pulse. --- .../xcschemes/Clock Signal.xcscheme | 2 + .../Bridges/DigitalPhaseLockedLoopBridge.h | 2 +- .../Bridges/DigitalPhaseLockedLoopBridge.mm | 8 +- .../Mac/Clock SignalTests/DPLLTests.swift | 8 +- Storage/Disk/Controller/DiskController.cpp | 15 ++- Storage/Disk/Controller/DiskController.hpp | 7 +- Storage/Disk/DPLL/DigitalPhaseLockedLoop.cpp | 55 ---------- Storage/Disk/DPLL/DigitalPhaseLockedLoop.hpp | 103 +++++++++++++++--- Storage/Disk/Track/TrackSerialiser.cpp | 4 +- Storage/Tape/Parsers/Acorn.cpp | 2 +- Storage/Tape/Parsers/Acorn.hpp | 4 +- 11 files changed, 115 insertions(+), 95 deletions(-) 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_; From 6df6af09de3e71c1cdf197e6a1dfb5236c9df024 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 12 Jan 2020 17:25:59 -0500 Subject: [PATCH 2/4] Remove dead .cpp. --- Storage/Disk/DPLL/DigitalPhaseLockedLoop.cpp | 14 -------------- 1 file changed, 14 deletions(-) delete mode 100644 Storage/Disk/DPLL/DigitalPhaseLockedLoop.cpp diff --git a/Storage/Disk/DPLL/DigitalPhaseLockedLoop.cpp b/Storage/Disk/DPLL/DigitalPhaseLockedLoop.cpp deleted file mode 100644 index 2e4ee02b7..000000000 --- a/Storage/Disk/DPLL/DigitalPhaseLockedLoop.cpp +++ /dev/null @@ -1,14 +0,0 @@ -// -// DigitalPhaseLockedLoop.cpp -// Clock Signal -// -// Created by Thomas Harte on 11/07/2016. -// Copyright 2016 Thomas Harte. All rights reserved. -// - -#include "DigitalPhaseLockedLoop.hpp" -#include -#include - -using namespace Storage; - From 8e3a6186194e7d60e60263a87562c240a8a5e955 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 12 Jan 2020 17:33:34 -0500 Subject: [PATCH 3/4] Corrects Mac build, shrinks default history [back] to 3 slots. --- OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj | 8 -------- Storage/Disk/DPLL/DigitalPhaseLockedLoop.hpp | 2 +- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj b/OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj index b6332489b..b9668ebc8 100644 --- a/OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj +++ b/OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj @@ -39,7 +39,6 @@ 4B055AA11FAE85DA0060FFFF /* OricMFMDSK.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4B4518971F75FD1B00926311 /* OricMFMDSK.cpp */; }; 4B055AA21FAE85DA0060FFFF /* SSD.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4B4518991F75FD1B00926311 /* SSD.cpp */; }; 4B055AA31FAE85DF0060FFFF /* ImplicitSectors.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4BFDD78B1F7F2DB4008579B9 /* ImplicitSectors.cpp */; }; - 4B055AA41FAE85E50060FFFF /* DigitalPhaseLockedLoop.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4B45187F1F75E91900926311 /* DigitalPhaseLockedLoop.cpp */; }; 4B055AA51FAE85EF0060FFFF /* Encoder.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4B7136841F78724F008B8ED9 /* Encoder.cpp */; }; 4B055AA61FAE85EF0060FFFF /* Parser.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4B71368C1F788112008B8ED9 /* Parser.cpp */; }; 4B055AA71FAE85EF0060FFFF /* SegmentParser.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4B71368F1F789C93008B8ED9 /* SegmentParser.cpp */; }; @@ -189,7 +188,6 @@ 4B4518841F75E91A00926311 /* UnformattedTrack.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4B4518771F75E91800926311 /* UnformattedTrack.cpp */; }; 4B4518851F75E91A00926311 /* DiskController.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4B45187A1F75E91900926311 /* DiskController.cpp */; }; 4B4518861F75E91A00926311 /* MFMDiskController.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4B45187C1F75E91900926311 /* MFMDiskController.cpp */; }; - 4B4518871F75E91A00926311 /* DigitalPhaseLockedLoop.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4B45187F1F75E91900926311 /* DigitalPhaseLockedLoop.cpp */; }; 4B45189F1F75FD1C00926311 /* AcornADF.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4B45188D1F75FD1B00926311 /* AcornADF.cpp */; }; 4B4518A01F75FD1C00926311 /* CPCDSK.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4B45188F1F75FD1B00926311 /* CPCDSK.cpp */; }; 4B4518A11F75FD1C00926311 /* D64.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4B4518911F75FD1B00926311 /* D64.cpp */; }; @@ -248,7 +246,6 @@ 4B778EEF23A5D6680000D260 /* AsyncTaskQueue.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4B3940E51DA83C8300427841 /* AsyncTaskQueue.cpp */; }; 4B778EF023A5D68C0000D260 /* 68000Storage.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4BFF1D3822337B0300838EA1 /* 68000Storage.cpp */; }; 4B778EF123A5D6B50000D260 /* 9918.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4B0E04F91FC9FA3100F43484 /* 9918.cpp */; }; - 4B778EF223A5DB100000D260 /* DigitalPhaseLockedLoop.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4B45187F1F75E91900926311 /* DigitalPhaseLockedLoop.cpp */; }; 4B778EF323A5DB230000D260 /* PCMSegment.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4B4518731F75E91800926311 /* PCMSegment.cpp */; }; 4B778EF423A5DB3A0000D260 /* C1540.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4B8334941F5E25B60097E338 /* C1540.cpp */; }; 4B778EF523A5DB440000D260 /* StaticAnalyser.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4B894517201967B4007DE474 /* StaticAnalyser.cpp */; }; @@ -1063,7 +1060,6 @@ 4B45187B1F75E91900926311 /* DiskController.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = DiskController.hpp; sourceTree = ""; }; 4B45187C1F75E91900926311 /* MFMDiskController.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = MFMDiskController.cpp; sourceTree = ""; }; 4B45187D1F75E91900926311 /* MFMDiskController.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = MFMDiskController.hpp; sourceTree = ""; }; - 4B45187F1F75E91900926311 /* DigitalPhaseLockedLoop.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = DigitalPhaseLockedLoop.cpp; sourceTree = ""; }; 4B4518801F75E91900926311 /* DigitalPhaseLockedLoop.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = DigitalPhaseLockedLoop.hpp; sourceTree = ""; }; 4B4518881F75ECB100926311 /* Track.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = Track.hpp; sourceTree = ""; }; 4B45188B1F75FD1B00926311 /* DiskImage.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = DiskImage.hpp; sourceTree = ""; }; @@ -2235,7 +2231,6 @@ 4B45187E1F75E91900926311 /* DPLL */ = { isa = PBXGroup; children = ( - 4B45187F1F75E91900926311 /* DigitalPhaseLockedLoop.cpp */, 4B4518801F75E91900926311 /* DigitalPhaseLockedLoop.hpp */, ); path = DPLL; @@ -4214,7 +4209,6 @@ 4B055A9A1FAE85CB0060FFFF /* MFMDiskController.cpp in Sources */, 4B0ACC3123775819008902D0 /* TIASound.cpp in Sources */, 4B055ACB1FAE9AFB0060FFFF /* SerialBus.cpp in Sources */, - 4B055AA41FAE85E50060FFFF /* DigitalPhaseLockedLoop.cpp in Sources */, 4B8318B122D3E53A006DB630 /* DiskIICard.cpp in Sources */, 4B055A9B1FAE85DA0060FFFF /* AcornADF.cpp in Sources */, 4B0E04F11FC9EA9500F43484 /* MSX.cpp in Sources */, @@ -4542,7 +4536,6 @@ 4B89449520194CB3007DE474 /* MachineForTarget.cpp in Sources */, 4B4A76301DB1A3FA007AAE2E /* AY38910.cpp in Sources */, 4B6A4C991F58F09E00E3F787 /* 6502Base.cpp in Sources */, - 4B4518871F75E91A00926311 /* DigitalPhaseLockedLoop.cpp in Sources */, 4B98A05E1FFAD3F600ADF63B /* CSROMFetcher.mm in Sources */, 4B7F188E2154825E00388727 /* MasterSystem.cpp in Sources */, 4B8805F41DCFD22A003085B1 /* Commodore.cpp in Sources */, @@ -4642,7 +4635,6 @@ 4B778F0E23A5EC4F0000D260 /* Tape.cpp in Sources */, 4B778F2D23A5EF190000D260 /* MFMDiskController.cpp in Sources */, 4B778F2723A5EEF60000D260 /* BinaryDump.cpp in Sources */, - 4B778EF223A5DB100000D260 /* DigitalPhaseLockedLoop.cpp in Sources */, 4BFCA1241ECBDCB400AC40C1 /* AllRAMProcessor.cpp in Sources */, 4B778F5223A5F22F0000D260 /* StaticAnalyser.cpp in Sources */, 4B778F4923A5F1F40000D260 /* StaticAnalyser.cpp in Sources */, diff --git a/Storage/Disk/DPLL/DigitalPhaseLockedLoop.hpp b/Storage/Disk/DPLL/DigitalPhaseLockedLoop.hpp index 41c25ed07..cece24d9d 100644 --- a/Storage/Disk/DPLL/DigitalPhaseLockedLoop.hpp +++ b/Storage/Disk/DPLL/DigitalPhaseLockedLoop.hpp @@ -23,7 +23,7 @@ namespace Storage { @c length_of_history The number of historic pulses to consider in locking to phase. */ -template class DigitalPhaseLockedLoop { +template class DigitalPhaseLockedLoop { public: /*! Instantiates a @c DigitalPhaseLockedLoop. From 514141f8c5b3751bc24009af93567a96033faa89 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 12 Jan 2020 17:45:02 -0500 Subject: [PATCH 4/4] Eliminates the optionality of a DPLL receiver. --- .../xcschemes/Clock Signal.xcscheme | 2 -- Storage/Disk/Controller/DiskController.cpp | 3 +-- Storage/Disk/DPLL/DigitalPhaseLockedLoop.hpp | 26 +++++++------------ Storage/Disk/Track/TrackSerialiser.cpp | 7 ++--- Storage/Tape/Parsers/Acorn.cpp | 6 ++--- 5 files changed, 16 insertions(+), 28 deletions(-) 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 81296a36f..47f9c7286 100644 --- a/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme +++ b/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme @@ -70,9 +70,7 @@ 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/Storage/Disk/Controller/DiskController.cpp b/Storage/Disk/Controller/DiskController.cpp index 8146a23be..8c69c1335 100644 --- a/Storage/Disk/Controller/DiskController.cpp +++ b/Storage/Disk/Controller/DiskController.cpp @@ -15,9 +15,8 @@ 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), + pll_(100, *this), empty_drive_(new Drive(int(clock_rate.as_integral()), 1, 1)) { - pll_.set_delegate(this); set_expected_bit_length(Time(1)); set_drive(empty_drive_); } diff --git a/Storage/Disk/DPLL/DigitalPhaseLockedLoop.hpp b/Storage/Disk/DPLL/DigitalPhaseLockedLoop.hpp index cece24d9d..b81643149 100644 --- a/Storage/Disk/DPLL/DigitalPhaseLockedLoop.hpp +++ b/Storage/Disk/DPLL/DigitalPhaseLockedLoop.hpp @@ -21,6 +21,7 @@ namespace Storage { /*! Template parameters: + @c bit_handler A class that must implement a method, digital_phase_locked_loop_output_bit(int) for receving bits from the DPLL. @c length_of_history The number of historic pulses to consider in locking to phase. */ template class DigitalPhaseLockedLoop { @@ -30,8 +31,8 @@ template class DigitalPhaseL @param clocks_per_bit The expected number of cycles between each bit of input. */ - DigitalPhaseLockedLoop(int clocks_per_bit) : - window_length_(clocks_per_bit), clocks_per_bit_(clocks_per_bit) {} + DigitalPhaseLockedLoop(int clocks_per_bit, BitHandler &handler) : + bit_handler_(handler), window_length_(clocks_per_bit), clocks_per_bit_(clocks_per_bit) {} /*! Changes the expected window length. @@ -51,12 +52,10 @@ template class DigitalPhaseL 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); - } + // Check whether this triggers any 0s. + if(window_was_filled_) --windows_crossed; + for(int c = 0; c < windows_crossed; c++) + bit_handler_.digital_phase_locked_loop_output_bit(0); window_was_filled_ = false; phase_ %= window_length_; @@ -68,22 +67,15 @@ template class DigitalPhaseL */ void add_pulse() { if(!window_was_filled_) { - if(delegate_) delegate_->digital_phase_locked_loop_output_bit(1); + bit_handler_.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. - */ - void set_delegate(BitHandler *delegate) { - delegate_ = delegate; - } - private: - BitHandler *delegate_ = nullptr; + BitHandler &bit_handler_; void post_phase_offset(Cycles::IntType new_phase, Cycles::IntType new_offset) { // Erase the effect of whatever is currently in this slot. diff --git a/Storage/Disk/Track/TrackSerialiser.cpp b/Storage/Disk/Track/TrackSerialiser.cpp index f8590fd6c..1ac070127 100644 --- a/Storage/Disk/Track/TrackSerialiser.cpp +++ b/Storage/Disk/Track/TrackSerialiser.cpp @@ -20,12 +20,13 @@ Storage::Disk::PCMSegment Storage::Disk::track_serialisation(const Track &track, // its PCMSegment. struct ResultAccumulator { PCMSegment result; + bool is_recording = false; void digital_phase_locked_loop_output_bit(int value) { - result.data.push_back(!!value); + if(is_recording) result.data.push_back(!!value); } } result_accumulator; result_accumulator.result.length_of_a_bit = length_of_a_bit; - DigitalPhaseLockedLoop pll(100); + DigitalPhaseLockedLoop pll(100, result_accumulator); // Obtain a length multiplier which is 100 times the reciprocal // of the expected bit length. So a perfect bit length from @@ -55,7 +56,7 @@ Storage::Disk::PCMSegment Storage::Disk::track_serialisation(const Track &track, if(!history_size) { track_copy->seek_to(Time(0)); time_error.set_zero(); - pll.set_delegate(&result_accumulator); + result_accumulator.is_recording = true; } } } diff --git a/Storage/Tape/Parsers/Acorn.cpp b/Storage/Tape/Parsers/Acorn.cpp index 1997f4d5a..46d3b0516 100644 --- a/Storage/Tape/Parsers/Acorn.cpp +++ b/Storage/Tape/Parsers/Acorn.cpp @@ -66,13 +66,11 @@ void Parser::process_pulse(const Storage::Tape::Tape::Pulse &pulse) { Shifter::Shifter() : - pll_(PLLClockRate / 4800), + pll_(PLLClockRate / 4800, *this), was_high_(false), input_pattern_(0), input_bit_counter_(0), - delegate_(nullptr) { - pll_.set_delegate(this); -} + delegate_(nullptr) {} void Shifter::process_pulse(const Storage::Tape::Tape::Pulse &pulse) { pll_.run_for(Cycles(static_cast(static_cast(PLLClockRate) * pulse.length.get())));