Switched to a much-more straightforward PLL. I think I'm just fiddling now rather than moving forwards. Probably time to move on?

OSBindings/Mac/Clock SignalTests/DPLLTests.swift

@@ -40,13 +40,13 @@ class DPLLTests: XCTestCase {
 		testRegularNibblesOnPLL(pll, bitLength: 110)
 	}
 
-	func testFivePercentSinePattern() {
+	func testTwentyPercentSinePattern() {
 		let pll = DigitalPhaseLockedLoopBridge(clocksPerBit: 100, tolerance: 20, historyLength: 3)
 		var angle = 0.0
 
 		// clock in two 1s, a 0, and a 1, 200 times over
 		for _ in 0 ..< 200 {
-			let bitLength: UInt = UInt(100 + 2 * sin(angle))
+			let bitLength: UInt = UInt(100 + 20 * sin(angle))
 
 			pll.runForCycles(bitLength/2)
 			pll.addPulse()

Storage/Disk/DigitalPhaseLockedLoop.cpp

@@ -12,88 +12,65 @@
 
 using namespace Storage;
 
-DigitalPhaseLockedLoop::DigitalPhaseLockedLoop(int clocks_per_bit, int tolerance, int length_of_history) :
+DigitalPhaseLockedLoop::DigitalPhaseLockedLoop(int clocks_per_bit, int tolerance, size_t length_of_history) :
 	_clocks_per_bit(clocks_per_bit),
 	_tolerance(tolerance),
-	_length_of_history(length_of_history),
+
-	_pulse_history(new int[length_of_history]),
+	_phase(0),
-	_current_window_length(clocks_per_bit),
+	_window_length(clocks_per_bit),
-	_next_pulse_time(0),
+
-	_window_was_filled(false),
+	_phase_error_pointer(0)
-	_window_offset(0),
+{
-	_samples_collected(0) {}
+	_phase_error_history.reset(new std::vector<int>(length_of_history, 0));
+}
 
 void DigitalPhaseLockedLoop::run_for_cycles(int number_of_cycles)
 {
 	// check whether this triggers any 0s
-	_window_offset += number_of_cycles;
+	_phase += number_of_cycles;
 	if(_delegate)
 	{
-		while(_window_offset > _current_window_length)
+		while(_phase > _window_length)
 		{
 			if(!_window_was_filled) _delegate->digital_phase_locked_loop_output_bit(0);
 			_window_was_filled = false;
-			_window_offset -= _current_window_length;
+			_phase -= _window_length;
 		}
 	}
 	else
 	{
-		_window_offset %= _current_window_length;
+		_phase %= _window_length;
 	}
-
-	// update timing
-	_next_pulse_time += number_of_cycles;
 }
 
 void DigitalPhaseLockedLoop::add_pulse()
 {
-	int *const _pulse_history_array = (int *)_pulse_history.get();
+	if(!_window_was_filled)
-	int outgoing_pulse_time = 0;
-
-	if(_samples_collected <= _length_of_history)
 	{
-		_samples_collected++;
-	}
-	else
-	{
-		outgoing_pulse_time	= _pulse_history_array[0];
-
-		// temporary: perform an exhaustive search for the ideal window length
-		int minimum_error = __INT_MAX__;
-		int ideal_length = 0;
-		for(int c = _clocks_per_bit - _tolerance; c < _clocks_per_bit + _tolerance; c++)
-		{
-			int total_error = 0;
-			const int half_window = c >> 1;
-			for(size_t pulse = 1; pulse < _length_of_history; pulse++)
-			{
-				int difference = _pulse_history_array[pulse] - _pulse_history_array[pulse-1];
-				difference += half_window;
-				const int steps = difference / c;
-				const int offset = difference%c - half_window;
-
-				total_error += abs(offset / steps);
-			}
-			if(total_error < minimum_error)
-			{
-				minimum_error = total_error;
-				ideal_length = c;
-			}
-		}
-
-		// use a spring mechanism to effect a lowpass filter
-		_current_window_length = ((ideal_length + (_current_window_length*3)) + 2) >> 2;
-	}
-
-	// therefore, there was a 1 in this window
-	_window_was_filled = true;
 		if(_delegate) _delegate->digital_phase_locked_loop_output_bit(1);
+		_window_was_filled = true;
+		post_phase_error(_phase - (_window_length >> 1));
+	}
+}
 
-	// shift history one to the left, storing new value, potentially with a centring adjustment
+void DigitalPhaseLockedLoop::post_phase_error(int error)
-	for(size_t pulse = 1; pulse < _length_of_history; pulse++)
 {
-		_pulse_history_array[pulse - 1] = _pulse_history_array[pulse] - outgoing_pulse_time;
+	// use a simple spring mechanism as a lowpass filter for phase
+	_phase -= (error + 1) >> 1;
+
+	// use the average of the last few errors to affect frequency
+	std::vector<int> *phase_error_history = _phase_error_history.get();
+	size_t phase_error_history_size = phase_error_history->size();
+
+	(*phase_error_history)[_phase_error_pointer] = error;
+	_phase_error_pointer = (_phase_error_pointer + 1)%phase_error_history_size;
+
+	int total_error = 0;
+	for(size_t c = 0; c < phase_error_history_size; c++)
+	{
+		total_error += (*phase_error_history)[c];
 	}
-	_next_pulse_time -= outgoing_pulse_time;
+	int denominator = (int)(phase_error_history_size * 4);
-	_pulse_history_array[_length_of_history-1] = _next_pulse_time;
+	_window_length += (total_error + (denominator >> 1)) / denominator;
+	_window_length = std::max(std::min(_window_length, _clocks_per_bit + _tolerance), _clocks_per_bit - _tolerance);
 }

Storage/Disk/DigitalPhaseLockedLoop.hpp

@@ -10,6 +10,7 @@
 #define DigitalPhaseLockedLoop_hpp
 
 #include <memory>
+#include <vector>
 
 namespace Storage {
 
@@ -22,7 +23,7 @@ class DigitalPhaseLockedLoop {
 			@param tolerance The maximum tolerance for bit windows — extremes will be clocks_per_bit ± tolerance.
 			@param length_of_history The number of historic pulses to consider in locking to phase.
 		*/
-		DigitalPhaseLockedLoop(int clocks_per_bit, int tolerance, int length_of_history);
+		DigitalPhaseLockedLoop(int clocks_per_bit, int tolerance, size_t length_of_history);
 
 		/*!
 			Runs the loop, impliedly posting no pulses during that period.
@@ -51,13 +52,12 @@ class DigitalPhaseLockedLoop {
 	private:
 		Delegate *_delegate;
 
-		std::unique_ptr<int> _pulse_history;
+		void post_phase_error(int error);
-		int _current_window_length;
+		std::unique_ptr<std::vector<int>> _phase_error_history;
-		int _length_of_history;
+		size_t _phase_error_pointer;
-		int _samples_collected;
 
-		int _next_pulse_time;
+		int _phase;
-		int _window_offset;
+		int _window_length;
 		bool _window_was_filled;
 
 		int _clocks_per_bit;