From 901f19f89c6c80da1821ccb94fe48028dfd3b9d7 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 25 Dec 2016 09:46:12 -0500 Subject: [PATCH 01/52] Added enough stuff that SSDs attached to a 1770 will now reach the entry point for writing. --- Components/1770/1770.cpp | 2 +- Storage/Disk/DiskController.cpp | 6 ++++++ Storage/Disk/DiskController.hpp | 1 + Storage/Disk/Formats/SSD.cpp | 5 +++++ Storage/Disk/Formats/SSD.hpp | 1 + 5 files changed, 14 insertions(+), 1 deletion(-) diff --git a/Components/1770/1770.cpp b/Components/1770/1770.cpp index 26434abb6..807af746a 100644 --- a/Components/1770/1770.cpp +++ b/Components/1770/1770.cpp @@ -498,7 +498,7 @@ void WD1770::posit_event(Event new_event_type) WAIT_FOR_TIME(30); test_type2_write_protection: - if(command_&0x20) // TODO:: && is_write_protected + if(command_&0x20 && get_drive_is_read_only()) { update_status([] (Status &status) { status.write_protect = true; diff --git a/Storage/Disk/DiskController.cpp b/Storage/Disk/DiskController.cpp index 07c655f47..fe7378129 100644 --- a/Storage/Disk/DiskController.cpp +++ b/Storage/Disk/DiskController.cpp @@ -181,6 +181,12 @@ bool Controller::get_drive_is_ready() return drive_->has_disk(); } +bool Controller::get_drive_is_read_only() +{ + if(!drive_) return false; + return drive_->get_is_read_only(); +} + void Controller::step(int direction) { if(drive_) drive_->step(direction); diff --git a/Storage/Disk/DiskController.hpp b/Storage/Disk/DiskController.hpp index ca856fa99..367259776 100644 --- a/Storage/Disk/DiskController.hpp +++ b/Storage/Disk/DiskController.hpp @@ -99,6 +99,7 @@ class Controller: public DigitalPhaseLockedLoop::Delegate, public TimedEventLoop bool get_is_track_zero(); void step(int direction); virtual bool get_drive_is_ready(); + bool get_drive_is_read_only(); private: Time bit_length_; diff --git a/Storage/Disk/Formats/SSD.cpp b/Storage/Disk/Formats/SSD.cpp index af4661dc1..3eea47728 100644 --- a/Storage/Disk/Formats/SSD.cpp +++ b/Storage/Disk/Formats/SSD.cpp @@ -40,6 +40,11 @@ unsigned int SSD::get_head_count() return head_count_; } +bool SSD::get_is_read_only() +{ + return false; +} + std::shared_ptr SSD::get_uncached_track_at_position(unsigned int head, unsigned int position) { std::shared_ptr track; diff --git a/Storage/Disk/Formats/SSD.hpp b/Storage/Disk/Formats/SSD.hpp index d75475370..90febd2cb 100644 --- a/Storage/Disk/Formats/SSD.hpp +++ b/Storage/Disk/Formats/SSD.hpp @@ -35,6 +35,7 @@ class SSD: public Disk, public Storage::FileHolder { // implemented to satisfy @c Disk unsigned int get_head_position_count(); unsigned int get_head_count(); + bool get_is_read_only(); private: std::shared_ptr get_uncached_track_at_position(unsigned int head, unsigned int position); From aceb7e3b6b91b0899e4a29cfcbc19323b51996e2 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 25 Dec 2016 12:31:38 -0500 Subject: [PATCH 02/52] Started implementing write sector on the 1770, immediately deciding it would be useful to have a callback for end-of-queued-data-written from disk controller. So had a go at implementing that, naively. More investigation required. --- Components/1770/1770.cpp | 38 +++++++++++++++++++++++++++++++-- Storage/Disk/DiskController.cpp | 33 +++++++++++++++++++++++++++- Storage/Disk/DiskController.hpp | 11 +++++++++- 3 files changed, 78 insertions(+), 4 deletions(-) diff --git a/Components/1770/1770.cpp b/Components/1770/1770.cpp index 807af746a..f739115fb 100644 --- a/Components/1770/1770.cpp +++ b/Components/1770/1770.cpp @@ -11,6 +11,8 @@ using namespace WD; +unsigned int counter = 0; + WD1770::Status::Status() : type(Status::One), write_protect(false), @@ -74,7 +76,12 @@ void WD1770::set_register(int address, uint8_t value) break; case 1: track_ = value; break; case 2: sector_ = value; break; - case 3: data_ = value; break; + case 3: + data_ = value; + update_status([] (Status &status) { + status.data_request = false; + }); + break; } } @@ -136,6 +143,7 @@ uint8_t WD1770::get_register(int address) void WD1770::run_for_cycles(unsigned int number_of_cycles) { + counter += number_of_cycles; Storage::Disk::Controller::run_for_cycles((int)number_of_cycles); if(delay_time_) @@ -288,6 +296,7 @@ void WD1770::process_index_hole() #define WAIT_FOR_EVENT(mask) resume_point_ = __LINE__; interesting_event_mask_ = mask; return; case __LINE__: #define WAIT_FOR_TIME(ms) resume_point_ = __LINE__; interesting_event_mask_ = Event::Timer; delay_time_ = ms * 8000; if(delay_time_) return; case __LINE__: +#define WAIT_FOR_BYTES(count) resume_point_ = __LINE__; interesting_event_mask_ = Event::Token; distance_into_section_ = 0; return; case __LINE__: if(latest_token_.type == Token::Byte) distance_into_section_++; if(distance_into_section_ < count) { interesting_event_mask_ = Event::Token; return; } #define BEGIN_SECTION() switch(resume_point_) { default: #define END_SECTION() 0; } @@ -586,7 +595,32 @@ void WD1770::posit_event(Event new_event_type) type2_write_data: - printf("!!!TODO: data portion of sector!!!\n"); + WAIT_FOR_BYTES(2); + update_status([] (Status &status) { + status.data_request = true; + }); + WAIT_FOR_BYTES(9); + if(status_.data_request) + { + update_status([] (Status &status) { + status.lost_data = true; + }); + goto wait_for_command; + } + WAIT_FOR_BYTES(1); + if(is_double_density_) + { + WAIT_FOR_BYTES(11); + } + + begin_writing(); + for(int c = 0; c < (is_double_density_ ? 12 : 6); c++) + { + // zero is encoded the same way in both FM and MFM + for(int b = 0; b < 16; b++) + write_bit(!(b&1)); + } + end_writing(); begin_type_3: update_status([] (Status &status) { diff --git a/Storage/Disk/DiskController.cpp b/Storage/Disk/DiskController.cpp index fe7378129..a8f0d4991 100644 --- a/Storage/Disk/DiskController.cpp +++ b/Storage/Disk/DiskController.cpp @@ -7,6 +7,7 @@ // #include "DiskController.hpp" +#include "../../NumberTheory/Factors.hpp" using namespace Storage::Disk; @@ -55,6 +56,8 @@ void Controller::setup_track() void Controller::run_for_cycles(int number_of_cycles) { + Time zero(0); + if(drive_ && drive_->has_disk() && motor_is_on_) { if(!track_) setup_track(); @@ -64,11 +67,31 @@ void Controller::run_for_cycles(int number_of_cycles) { int cycles_until_next_event = (int)get_cycles_until_next_event(); int cycles_to_run_for = std::min(cycles_until_next_event, number_of_cycles); + if(!is_reading_ && cycles_until_bits_written_ > zero) cycles_to_run_for = std::min(cycles_to_run_for, (int)cycles_until_bits_written_.get_unsigned_int()); cycles_since_index_hole_ += (unsigned int)cycles_to_run_for; number_of_cycles -= cycles_to_run_for; - if(is_reading_) pll_->run_for_cycles(cycles_to_run_for); + if(is_reading_) + { + pll_->run_for_cycles(cycles_to_run_for); + } + else + { + if(cycles_until_bits_written_ > Storage::Time(0)) + { + Storage::Time number_of_cycles_time(number_of_cycles); + if(cycles_until_bits_written_ < number_of_cycles_time) + { + cycles_until_bits_written_.set_zero(); + process_write_completed(); + } + else + { + cycles_until_bits_written_ -= number_of_cycles_time; + } + } + } TimedEventLoop::run_for_cycles(cycles_to_run_for); } } @@ -136,6 +159,8 @@ void Controller::write_bit(bool value) if(needs_new_byte) write_segment_.data.push_back(0); if(value) write_segment_.data[write_segment_.number_of_bits >> 3] |= 0x80 >> (write_segment_.number_of_bits & 7); write_segment_.number_of_bits++; + + cycles_until_bits_written_ += cycles_per_bit_; } void Controller::end_writing() @@ -155,6 +180,8 @@ void Controller::set_expected_bit_length(Time bit_length) { bit_length_ = bit_length; + cycles_per_bit_ = Storage::Time(8000000) * (bit_length * rotational_multiplier_); + // 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 = (int)((bit_length.length * clock_rate_) / bit_length.clock_rate); @@ -213,3 +240,7 @@ void Controller::invalidate_track() { track_ = nullptr; } + +void Controller::process_write_completed() +{ +} diff --git a/Storage/Disk/DiskController.hpp b/Storage/Disk/DiskController.hpp index 367259776..1ab09adde 100644 --- a/Storage/Disk/DiskController.hpp +++ b/Storage/Disk/DiskController.hpp @@ -86,10 +86,16 @@ class Controller: public DigitalPhaseLockedLoop::Delegate, public TimedEventLoop virtual void process_input_bit(int value, unsigned int cycles_since_index_hole) = 0; /*! - Should be implemented by subcalsses; communicates that the index hole has been reached. + Should be implemented by subclasses; communicates that the index hole has been reached. */ virtual void process_index_hole() = 0; + /*! + Should be implemented by subclasses if they implement writing; communicates that + all bits supplied to write_bit have now been written. + */ + virtual void process_write_completed(); + // for TimedEventLoop virtual void process_next_event(); @@ -122,6 +128,9 @@ class Controller: public DigitalPhaseLockedLoop::Delegate, public TimedEventLoop PCMSegment write_segment_; Time write_start_time_; + Time cycles_until_bits_written_; + Time cycles_per_bit_; + void setup_track(); Time get_time_into_track(); }; From ec55a25620bb2ce545c63f123a7dfbb5002fe5d1 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 25 Dec 2016 12:32:25 -0500 Subject: [PATCH 03/52] It makes sense to simplify these ahead of time. --- Storage/Disk/DiskController.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Storage/Disk/DiskController.cpp b/Storage/Disk/DiskController.cpp index a8f0d4991..18bb0a1a3 100644 --- a/Storage/Disk/DiskController.cpp +++ b/Storage/Disk/DiskController.cpp @@ -179,8 +179,10 @@ void Controller::end_writing() void Controller::set_expected_bit_length(Time bit_length) { bit_length_ = bit_length; + bit_length_.simplify(); cycles_per_bit_ = Storage::Time(8000000) * (bit_length * rotational_multiplier_); + cycles_per_bit_.simplify(); // 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 From d2ad2c756e1488b50d2a796177bc6dc8c6ce7b5a Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 25 Dec 2016 15:46:49 -0500 Subject: [PATCH 04/52] Added enough shovelling to write rubbish for an entire sector. --- Components/1770/1770.cpp | 45 ++++++++++++++++++++++++++ Components/1770/1770.hpp | 6 ++-- Storage/Disk/DiskController.cpp | 20 ++++++++---- Storage/Storage.hpp | 56 ++++++++++++++++++++++++++++----- 4 files changed, 111 insertions(+), 16 deletions(-) diff --git a/Components/1770/1770.cpp b/Components/1770/1770.cpp index f739115fb..35c1d8c5b 100644 --- a/Components/1770/1770.cpp +++ b/Components/1770/1770.cpp @@ -277,6 +277,12 @@ void WD1770::process_index_hole() } } +void WD1770::process_write_completed() +{ + posit_event(Event::DataWritten); +} + + // +------+----------+-------------------------+ // ! ! ! BITS ! // ! TYPE ! COMMAND ! 7 6 5 4 3 2 1 0 ! @@ -620,8 +626,47 @@ void WD1770::posit_event(Event new_event_type) for(int b = 0; b < 16; b++) write_bit(!(b&1)); } +// printf("%d\n", counter); + WAIT_FOR_EVENT(Event::DataWritten); + distance_into_section_ = 0; + + type2_write_loop: + // TODO: real data + for(int b = 0; b < 16; b++) + write_bit(!(b&1)); + update_status([] (Status &status) { + status.data_request = true; + }); + WAIT_FOR_EVENT(Event::DataWritten); + distance_into_section_++; + if(distance_into_section_ == 128 << header_[3]) + { + goto type2_write_crc; + } + + if(status_.data_request) + { + update_status([] (Status &status) { + status.lost_data = true; + }); + goto wait_for_command; + } + + type2_write_crc: + // TODO: write CRC and FF + for(int b = 0; b < 48; b++) + write_bit(!(b&1)); + WAIT_FOR_EVENT(Event::DataWritten); end_writing(); + if(command_ & 0x10) + { + sector_++; + goto test_type2_write_protection; + } + printf("Wrote sector %d\n", sector_); + goto wait_for_command; + begin_type_3: update_status([] (Status &status) { status.type = Status::Three; diff --git a/Components/1770/1770.hpp b/Components/1770/1770.hpp index edfe809e3..759423d5f 100644 --- a/Components/1770/1770.hpp +++ b/Components/1770/1770.hpp @@ -110,9 +110,10 @@ class WD1770: public Storage::Disk::Controller { Token = (1 << 1), // Indicates recognition of a new token in the flux stream. Interrogate latest_token_ for details. IndexHole = (1 << 2), // Indicates the passing of a physical index hole. HeadLoad = (1 << 3), // Indicates the head has been loaded (1973 only). + DataWritten = (1 << 4), // Indicates that all queued bits have been written - Timer = (1 << 4), // Indicates that the delay_time_-powered timer has timed out. - IndexHoleTarget = (1 << 5) // Indicates that index_hole_count_ has reached index_hole_count_target_. + Timer = (1 << 5), // Indicates that the delay_time_-powered timer has timed out. + IndexHoleTarget = (1 << 6) // Indicates that index_hole_count_ has reached index_hole_count_target_. }; void posit_event(Event type); int interesting_event_mask_; @@ -131,6 +132,7 @@ class WD1770: public Storage::Disk::Controller { // Storage::Disk::Controller virtual void process_input_bit(int value, unsigned int cycles_since_index_hole); virtual void process_index_hole(); + virtual void process_write_completed(); }; } diff --git a/Storage/Disk/DiskController.cpp b/Storage/Disk/DiskController.cpp index 18bb0a1a3..8507141bc 100644 --- a/Storage/Disk/DiskController.cpp +++ b/Storage/Disk/DiskController.cpp @@ -67,7 +67,12 @@ void Controller::run_for_cycles(int number_of_cycles) { int cycles_until_next_event = (int)get_cycles_until_next_event(); int cycles_to_run_for = std::min(cycles_until_next_event, number_of_cycles); - if(!is_reading_ && cycles_until_bits_written_ > zero) cycles_to_run_for = std::min(cycles_to_run_for, (int)cycles_until_bits_written_.get_unsigned_int()); + if(!is_reading_ && cycles_until_bits_written_ > zero) + { + int write_cycles_target = (int)cycles_until_bits_written_.get_unsigned_int(); + if(cycles_until_bits_written_.length % cycles_until_bits_written_.clock_rate) write_cycles_target++; + cycles_to_run_for = std::min(cycles_to_run_for, write_cycles_target); + } cycles_since_index_hole_ += (unsigned int)cycles_to_run_for; @@ -78,17 +83,20 @@ void Controller::run_for_cycles(int number_of_cycles) } else { - if(cycles_until_bits_written_ > Storage::Time(0)) + if(cycles_until_bits_written_ > zero) { - Storage::Time number_of_cycles_time(number_of_cycles); - if(cycles_until_bits_written_ < number_of_cycles_time) + Storage::Time cycles_to_run_for_time(cycles_to_run_for); + if(cycles_until_bits_written_ < cycles_to_run_for_time) { - cycles_until_bits_written_.set_zero(); process_write_completed(); + if(cycles_until_bits_written_ < cycles_to_run_for_time) + cycles_until_bits_written_.set_zero(); + else + cycles_until_bits_written_ -= cycles_to_run_for_time; } else { - cycles_until_bits_written_ -= number_of_cycles_time; + cycles_until_bits_written_ -= cycles_to_run_for_time; } } } diff --git a/Storage/Storage.hpp b/Storage/Storage.hpp index a0456870f..3feff4d5f 100644 --- a/Storage/Storage.hpp +++ b/Storage/Storage.hpp @@ -89,30 +89,70 @@ struct Time { inline Time operator + (const Time &other) const { - uint64_t result_length = (uint64_t)length * (uint64_t)other.clock_rate + (uint64_t)other.length * (uint64_t)clock_rate; - uint64_t result_clock_rate = (uint64_t)clock_rate * (uint64_t)other.clock_rate; + uint64_t result_length; + uint64_t result_clock_rate; + if(clock_rate == other.clock_rate) + { + result_length = (uint64_t)length + (uint64_t)other.length; + result_clock_rate = clock_rate; + } + else + { + result_length = (uint64_t)length * (uint64_t)other.clock_rate + (uint64_t)other.length * (uint64_t)clock_rate; + result_clock_rate = (uint64_t)clock_rate * (uint64_t)other.clock_rate; + } return Time(result_length, result_clock_rate); } inline Time &operator += (const Time &other) { - uint64_t result_length = (uint64_t)length * (uint64_t)other.clock_rate + (uint64_t)other.length * (uint64_t)clock_rate; - uint64_t result_clock_rate = (uint64_t)clock_rate * (uint64_t)other.clock_rate; + uint64_t result_length; + uint64_t result_clock_rate; + if(clock_rate == other.clock_rate) + { + result_length = (uint64_t)length + (uint64_t)other.length; + result_clock_rate = (uint64_t)clock_rate; + } + else + { + result_length = (uint64_t)length * (uint64_t)other.clock_rate + (uint64_t)other.length * (uint64_t)clock_rate; + result_clock_rate = (uint64_t)clock_rate * (uint64_t)other.clock_rate; + } install_result(result_length, result_clock_rate); return *this; } inline Time operator - (const Time &other) const { - uint64_t result_length = (uint64_t)length * (uint64_t)other.clock_rate - (uint64_t)other.length * (uint64_t)clock_rate; - uint64_t result_clock_rate = (uint64_t)clock_rate * (uint64_t)other.clock_rate; + uint64_t result_length; + uint64_t result_clock_rate; + if(clock_rate == other.clock_rate) + { + result_length = (uint64_t)length - (uint64_t)other.length; + result_clock_rate = clock_rate; + } + else + { + result_length = (uint64_t)length * (uint64_t)other.clock_rate - (uint64_t)other.length * (uint64_t)clock_rate; + result_clock_rate = (uint64_t)clock_rate * (uint64_t)other.clock_rate; + } return Time(result_length, result_clock_rate); } inline Time operator -= (const Time &other) { - uint64_t result_length = (uint64_t)length * (uint64_t)other.clock_rate - (uint64_t)other.length * (uint64_t)clock_rate; - uint64_t result_clock_rate = (uint64_t)clock_rate * (uint64_t)other.clock_rate; + uint64_t result_length; + uint64_t result_clock_rate; + if(clock_rate == other.clock_rate) + { + result_length = (uint64_t)length - (uint64_t)other.length; + result_clock_rate = (uint64_t)clock_rate; + } + else + { + result_length = (uint64_t)length * (uint64_t)other.clock_rate - (uint64_t)other.length * (uint64_t)clock_rate; + result_clock_rate = (uint64_t)clock_rate * (uint64_t)other.clock_rate; + } install_result(result_length, result_clock_rate); return *this; } From 98be6ede458f1da2a89ca036d5329e1e31fb2b15 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 25 Dec 2016 16:13:05 -0500 Subject: [PATCH 05/52] Shuffled a little to reduce risk of overflow, ensured writing is a loop, still seem to be writing too quickly for some reason. --- Components/1770/1770.cpp | 5 ++++- Storage/Disk/DiskController.cpp | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Components/1770/1770.cpp b/Components/1770/1770.cpp index 35c1d8c5b..8888bea69 100644 --- a/Components/1770/1770.cpp +++ b/Components/1770/1770.cpp @@ -626,7 +626,6 @@ void WD1770::posit_event(Event new_event_type) for(int b = 0; b < 16; b++) write_bit(!(b&1)); } -// printf("%d\n", counter); WAIT_FOR_EVENT(Event::DataWritten); distance_into_section_ = 0; @@ -637,7 +636,9 @@ void WD1770::posit_event(Event new_event_type) update_status([] (Status &status) { status.data_request = true; }); + printf("- %d\n", counter); WAIT_FOR_EVENT(Event::DataWritten); + printf("+ %d\n", counter); distance_into_section_++; if(distance_into_section_ == 128 << header_[3]) { @@ -652,6 +653,8 @@ void WD1770::posit_event(Event new_event_type) goto wait_for_command; } + goto type2_write_loop; + type2_write_crc: // TODO: write CRC and FF for(int b = 0; b < 48; b++) diff --git a/Storage/Disk/DiskController.cpp b/Storage/Disk/DiskController.cpp index 8507141bc..827d47b12 100644 --- a/Storage/Disk/DiskController.cpp +++ b/Storage/Disk/DiskController.cpp @@ -189,7 +189,7 @@ void Controller::set_expected_bit_length(Time bit_length) bit_length_ = bit_length; bit_length_.simplify(); - cycles_per_bit_ = Storage::Time(8000000) * (bit_length * rotational_multiplier_); + cycles_per_bit_ = Storage::Time(8000000) * (bit_length * rotational_multiplier_) * Storage::Time(clock_rate_multiplier_); cycles_per_bit_.simplify(); // this conversion doesn't need to be exact because there's a lot of variation to be taken From 007c13ec16b35f283c01842fb9135c99e68cd7aa Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 25 Dec 2016 16:35:39 -0500 Subject: [PATCH 06/52] Fixed: cycles_per_bit_ isn't a function of the rotational multiplier, it's absolute. Also made sure that exactly hitting the end of a bit counts. --- Storage/Disk/DiskController.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Storage/Disk/DiskController.cpp b/Storage/Disk/DiskController.cpp index 827d47b12..aa9765b81 100644 --- a/Storage/Disk/DiskController.cpp +++ b/Storage/Disk/DiskController.cpp @@ -86,10 +86,10 @@ void Controller::run_for_cycles(int number_of_cycles) if(cycles_until_bits_written_ > zero) { Storage::Time cycles_to_run_for_time(cycles_to_run_for); - if(cycles_until_bits_written_ < cycles_to_run_for_time) + if(cycles_until_bits_written_ <= cycles_to_run_for_time) { process_write_completed(); - if(cycles_until_bits_written_ < cycles_to_run_for_time) + if(cycles_until_bits_written_ <= cycles_to_run_for_time) cycles_until_bits_written_.set_zero(); else cycles_until_bits_written_ -= cycles_to_run_for_time; @@ -189,12 +189,12 @@ void Controller::set_expected_bit_length(Time bit_length) bit_length_ = bit_length; bit_length_.simplify(); - cycles_per_bit_ = Storage::Time(8000000) * (bit_length * rotational_multiplier_) * Storage::Time(clock_rate_multiplier_); + cycles_per_bit_ = Storage::Time(clock_rate_) * bit_length; cycles_per_bit_.simplify(); // 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 = (int)((bit_length.length * clock_rate_) / bit_length.clock_rate); + int clocks_per_bit = (int)cycles_per_bit_.get_unsigned_int(); pll_.reset(new DigitalPhaseLockedLoop(clocks_per_bit, clocks_per_bit / 5, 3)); pll_->set_delegate(this); } From 74e98fd097c21efb9fdc28666a73989757b95762 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 25 Dec 2016 19:18:45 -0500 Subject: [PATCH 07/52] Made an attempt to write actual data (albeit that CRC calculation is still missing). --- Components/1770/1770.cpp | 35 ++++++++++++++++++++++++++--------- Components/1770/1770.hpp | 5 +++++ 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/Components/1770/1770.cpp b/Components/1770/1770.cpp index 8888bea69..4f065a3e5 100644 --- a/Components/1770/1770.cpp +++ b/Components/1770/1770.cpp @@ -622,17 +622,13 @@ void WD1770::posit_event(Event new_event_type) begin_writing(); for(int c = 0; c < (is_double_density_ ? 12 : 6); c++) { - // zero is encoded the same way in both FM and MFM - for(int b = 0; b < 16; b++) - write_bit(!(b&1)); + write_byte(0); } WAIT_FOR_EVENT(Event::DataWritten); distance_into_section_ = 0; type2_write_loop: - // TODO: real data - for(int b = 0; b < 16; b++) - write_bit(!(b&1)); + write_byte(data_); update_status([] (Status &status) { status.data_request = true; }); @@ -656,9 +652,10 @@ void WD1770::posit_event(Event new_event_type) goto type2_write_loop; type2_write_crc: - // TODO: write CRC and FF - for(int b = 0; b < 48; b++) - write_bit(!(b&1)); + // TODO: write CRC + write_byte(0); + write_byte(0); + write_byte(0xff); WAIT_FOR_EVENT(Event::DataWritten); end_writing(); @@ -701,3 +698,23 @@ void WD1770::set_head_loaded(bool head_loaded) head_is_loaded_ = head_loaded; if(head_loaded) posit_event(Event::HeadLoad); } + +void WD1770::write_bit(int bit) +{ + if(is_double_density_) + { + Controller::write_bit(!bit && !last_bit_); + Controller::write_bit(!!bit); + last_bit_ = bit; + } + else + { + Controller::write_bit(true); + Controller::write_bit(!!bit); + } +} + +void WD1770::write_byte(uint8_t byte) +{ + for(int c = 0; c < 8; c++) write_bit((byte << c)&0x80); +} diff --git a/Components/1770/1770.hpp b/Components/1770/1770.hpp index 759423d5f..3aaefef88 100644 --- a/Components/1770/1770.hpp +++ b/Components/1770/1770.hpp @@ -120,6 +120,11 @@ class WD1770: public Storage::Disk::Controller { int resume_point_; int delay_time_; + // Output + int last_bit_; + void write_bit(int bit); + void write_byte(uint8_t byte); + // ID buffer uint8_t header_[6]; From 1349e85d83756f03ae54898f3f410248783587f6 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 25 Dec 2016 19:19:22 -0500 Subject: [PATCH 08/52] [Mostly] fixed track write-back. --- Storage/Disk/Disk.cpp | 2 +- Storage/Disk/DiskController.cpp | 18 ++++++++---------- Storage/Disk/DiskController.hpp | 1 - 3 files changed, 9 insertions(+), 12 deletions(-) diff --git a/Storage/Disk/Disk.cpp b/Storage/Disk/Disk.cpp index c6b04e0ee..96953f5bd 100644 --- a/Storage/Disk/Disk.cpp +++ b/Storage/Disk/Disk.cpp @@ -17,7 +17,7 @@ int Disk::get_id_for_track_at_position(unsigned int head, unsigned int position) void Disk::set_track_at_position(unsigned int head, unsigned int position, const std::shared_ptr &track) { - if(!get_is_read_only()) return; + if(get_is_read_only()) return; int address = get_id_for_track_at_position(head, position); cached_tracks_[address] = track; diff --git a/Storage/Disk/DiskController.cpp b/Storage/Disk/DiskController.cpp index aa9765b81..3aa3fa2cf 100644 --- a/Storage/Disk/DiskController.cpp +++ b/Storage/Disk/DiskController.cpp @@ -20,7 +20,6 @@ Controller::Controller(unsigned int clock_rate, unsigned int clock_rate_multipli motor_is_on_(false), is_reading_(true), - track_is_dirty_(false), TimedEventLoop(clock_rate * clock_rate_multiplier) { @@ -31,13 +30,7 @@ Controller::Controller(unsigned int clock_rate, unsigned int clock_rate_multipli void Controller::setup_track() { - if(patched_track_) - { - drive_->set_track(patched_track_); - } - track_ = drive_->get_track(); - track_is_dirty_ = false; Time offset; Time track_time_now = get_time_into_track(); @@ -154,7 +147,7 @@ void Controller::begin_writing() { is_reading_ = false; - write_segment_.length_of_a_bit = bit_length_ * rotational_multiplier_; + write_segment_.length_of_a_bit = bit_length_ / rotational_multiplier_; write_segment_.data.clear(); write_segment_.number_of_bits = 0; @@ -226,8 +219,8 @@ bool Controller::get_drive_is_read_only() void Controller::step(int direction) { - if(drive_) drive_->step(direction); invalidate_track(); + if(drive_) drive_->step(direction); } void Controller::set_motor_on(bool motor_on) @@ -242,13 +235,18 @@ bool Controller::get_motor_on() void Controller::set_drive(std::shared_ptr drive) { - drive_ = drive; invalidate_track(); + drive_ = drive; } void Controller::invalidate_track() { track_ = nullptr; + if(patched_track_) + { + drive_->set_track(patched_track_); + patched_track_ = nullptr; + } } void Controller::process_write_completed() diff --git a/Storage/Disk/DiskController.hpp b/Storage/Disk/DiskController.hpp index 1ab09adde..f309307bc 100644 --- a/Storage/Disk/DiskController.hpp +++ b/Storage/Disk/DiskController.hpp @@ -123,7 +123,6 @@ class Controller: public DigitalPhaseLockedLoop::Delegate, public TimedEventLoop bool motor_is_on_; bool is_reading_; - bool track_is_dirty_; std::shared_ptr patched_track_; PCMSegment write_segment_; Time write_start_time_; From beaa868079d7e3e763dbf513ae10a9403fef99c9 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 25 Dec 2016 20:00:57 -0500 Subject: [PATCH 09/52] Factored the MFM parser out into encodings. --- Components/1770/1770.cpp | 2 - StaticAnalyser/Acorn/Disk.cpp | 174 +-------------------------------- Storage/Disk/Encodings/MFM.cpp | 149 ++++++++++++++++++++++++++++ Storage/Disk/Encodings/MFM.hpp | 30 ++++++ 4 files changed, 181 insertions(+), 174 deletions(-) diff --git a/Components/1770/1770.cpp b/Components/1770/1770.cpp index 4f065a3e5..c0e2a1bca 100644 --- a/Components/1770/1770.cpp +++ b/Components/1770/1770.cpp @@ -632,9 +632,7 @@ void WD1770::posit_event(Event new_event_type) update_status([] (Status &status) { status.data_request = true; }); - printf("- %d\n", counter); WAIT_FOR_EVENT(Event::DataWritten); - printf("+ %d\n", counter); distance_into_section_++; if(distance_into_section_ == 128 << header_[3]) { diff --git a/StaticAnalyser/Acorn/Disk.cpp b/StaticAnalyser/Acorn/Disk.cpp index 20fa8a0a6..c7cf4b882 100644 --- a/StaticAnalyser/Acorn/Disk.cpp +++ b/StaticAnalyser/Acorn/Disk.cpp @@ -14,180 +14,11 @@ using namespace StaticAnalyser::Acorn; -class FMParser: public Storage::Disk::Controller { - public: - std::shared_ptr drive; - - FMParser(bool is_mfm) : - Storage::Disk::Controller(4000000, 1, 300), - crc_generator_(0x1021, 0xffff), - shift_register_(0), track_(0), is_mfm_(is_mfm) - { - Storage::Time bit_length; - bit_length.length = 1; - bit_length.clock_rate = is_mfm ? 500000 : 250000; // i.e. 250 kbps (including clocks) - set_expected_bit_length(bit_length); - - drive.reset(new Storage::Disk::Drive); - set_drive(drive); - set_motor_on(true); - } - - /*! - Attempts to read the sector located at @c track and @c sector. - - @returns a sector if one was found; @c nullptr otherwise. - */ - std::shared_ptr get_sector(uint8_t track, uint8_t sector) - { - int difference = (int)track - (int)track_; - track_ = track; - - if(difference) - { - int direction = difference < 0 ? -1 : 1; - difference *= direction; - - for(int c = 0; c < difference; c++) step(direction); - } - - return get_sector(sector); - } - - private: - unsigned int shift_register_; - int index_count_; - uint8_t track_; - int bit_count_; - std::shared_ptr sector_cache_[65536]; - NumberTheory::CRC16 crc_generator_; - bool is_mfm_; - - void process_input_bit(int value, unsigned int cycles_since_index_hole) - { - shift_register_ = ((shift_register_ << 1) | (unsigned int)value) & 0xffff; - bit_count_++; - } - - void process_index_hole() - { - index_count_++; - } - - uint8_t get_next_byte() - { - bit_count_ = 0; - while(bit_count_ < 16) run_for_cycles(1); - uint8_t byte = (uint8_t)( - ((shift_register_&0x0001) >> 0) | - ((shift_register_&0x0004) >> 1) | - ((shift_register_&0x0010) >> 2) | - ((shift_register_&0x0040) >> 3) | - ((shift_register_&0x0100) >> 4) | - ((shift_register_&0x0400) >> 5) | - ((shift_register_&0x1000) >> 6) | - ((shift_register_&0x4000) >> 7)); - crc_generator_.add(byte); - return byte; - } - - std::shared_ptr get_next_sector() - { - std::shared_ptr sector(new Storage::Encodings::MFM::Sector); - index_count_ = 0; - - while(index_count_ < 2) - { - // look for an ID address mark - while(1) - { - run_for_cycles(1); - if(is_mfm_) - { - if(shift_register_ == Storage::Encodings::MFM::MFMAddressMark) - { - uint8_t mark = get_next_byte(); - if(mark == Storage::Encodings::MFM::MFMIDAddressByte) break; - } - } - else - { - if(shift_register_ == Storage::Encodings::MFM::FMIDAddressMark) break; - } - if(index_count_ >= 2) return nullptr; - } - - crc_generator_.reset(); - sector->track = get_next_byte(); - sector->side = get_next_byte(); - sector->sector = get_next_byte(); - uint8_t size = get_next_byte(); - uint16_t header_crc = crc_generator_.get_value(); - if((header_crc >> 8) != get_next_byte()) continue; - if((header_crc & 0xff) != get_next_byte()) continue; - - // look for data mark - while(1) - { - run_for_cycles(1); - if(is_mfm_) - { - if(shift_register_ == Storage::Encodings::MFM::MFMAddressMark) - { - uint8_t mark = get_next_byte(); - if(mark == Storage::Encodings::MFM::MFMDataAddressByte) break; - if(mark == Storage::Encodings::MFM::MFMIDAddressByte) return nullptr; - } - } - else - { - if(shift_register_ == Storage::Encodings::MFM::FMDataAddressMark) break; - if(shift_register_ == Storage::Encodings::MFM::FMIDAddressMark) return nullptr; - } - if(index_count_ >= 2) return nullptr; - } - - size_t data_size = (size_t)(128 << size); - sector->data.reserve(data_size); - crc_generator_.reset(); - for(size_t c = 0; c < data_size; c++) - { - sector->data.push_back(get_next_byte()); - } - uint16_t data_crc = crc_generator_.get_value(); - if((data_crc >> 8) != get_next_byte()) continue; - if((data_crc & 0xff) != get_next_byte()) continue; - - return sector; - } - - return nullptr; - } - - std::shared_ptr get_sector(uint8_t sector) - { -// uint16_t sector_address = (uint16_t)((track_ << 8) | sector); -// if(sector_cache_[sector_address]) return sector_cache_[sector_address]; - - std::shared_ptr first_sector = get_next_sector(); - if(!first_sector) return first_sector; - if(first_sector->sector == sector) return first_sector; - - while(1) - { - std::shared_ptr next_sector = get_next_sector(); - if(next_sector->sector == first_sector->sector) return nullptr; - if(next_sector->sector == sector) return next_sector; - } - } -}; - std::unique_ptr StaticAnalyser::Acorn::GetDFSCatalogue(const std::shared_ptr &disk) { // c.f. http://beebwiki.mdfs.net/Acorn_DFS_disc_format std::unique_ptr catalogue(new Catalogue); - FMParser parser(false); - parser.drive->set_disk(disk); + Storage::Encodings::MFM::Parser parser(false, disk); std::shared_ptr names = parser.get_sector(0, 0); std::shared_ptr details = parser.get_sector(0, 1); @@ -248,8 +79,7 @@ std::unique_ptr StaticAnalyser::Acorn::GetDFSCatalogue(const std::sha std::unique_ptr StaticAnalyser::Acorn::GetADFSCatalogue(const std::shared_ptr &disk) { std::unique_ptr catalogue(new Catalogue); - FMParser parser(true); - parser.drive->set_disk(disk); + Storage::Encodings::MFM::Parser parser(true, disk); std::shared_ptr free_space_map_second_half = parser.get_sector(0, 1); if(!free_space_map_second_half) return nullptr; diff --git a/Storage/Disk/Encodings/MFM.cpp b/Storage/Disk/Encodings/MFM.cpp index 8c864ff53..2af4e5bfb 100644 --- a/Storage/Disk/Encodings/MFM.cpp +++ b/Storage/Disk/Encodings/MFM.cpp @@ -211,3 +211,152 @@ std::unique_ptr Storage::Encodings::MFM::GetFMEncoder(std::vector(new FMEncoder(target)); } + +#pragma mark - Parser + +Parser::Parser(bool is_mfm, const std::shared_ptr &disk) : + Storage::Disk::Controller(4000000, 1, 300), + crc_generator_(0x1021, 0xffff), + shift_register_(0), track_(0), is_mfm_(is_mfm) +{ + Storage::Time bit_length; + bit_length.length = 1; + bit_length.clock_rate = is_mfm ? 500000 : 250000; // i.e. 250 kbps (including clocks) + set_expected_bit_length(bit_length); + + drive.reset(new Storage::Disk::Drive); + drive->set_disk(disk); + set_drive(drive); + set_motor_on(true); +} + +std::shared_ptr Parser::get_sector(uint8_t track, uint8_t sector) +{ + int difference = (int)track - (int)track_; + track_ = track; + + if(difference) + { + int direction = difference < 0 ? -1 : 1; + difference *= direction; + + for(int c = 0; c < difference; c++) step(direction); + } + + return get_sector(sector); +} + +void Parser::process_input_bit(int value, unsigned int cycles_since_index_hole) +{ + shift_register_ = ((shift_register_ << 1) | (unsigned int)value) & 0xffff; + bit_count_++; +} + +void Parser::process_index_hole() +{ + index_count_++; +} + +uint8_t Parser::get_next_byte() +{ + bit_count_ = 0; + while(bit_count_ < 16) run_for_cycles(1); + uint8_t byte = (uint8_t)( + ((shift_register_&0x0001) >> 0) | + ((shift_register_&0x0004) >> 1) | + ((shift_register_&0x0010) >> 2) | + ((shift_register_&0x0040) >> 3) | + ((shift_register_&0x0100) >> 4) | + ((shift_register_&0x0400) >> 5) | + ((shift_register_&0x1000) >> 6) | + ((shift_register_&0x4000) >> 7)); + crc_generator_.add(byte); + return byte; +} + +std::shared_ptr Parser::get_next_sector() +{ + std::shared_ptr sector(new Storage::Encodings::MFM::Sector); + index_count_ = 0; + + while(index_count_ < 2) + { + // look for an ID address mark + while(1) + { + run_for_cycles(1); + if(is_mfm_) + { + if(shift_register_ == Storage::Encodings::MFM::MFMAddressMark) + { + uint8_t mark = get_next_byte(); + if(mark == Storage::Encodings::MFM::MFMIDAddressByte) break; + } + } + else + { + if(shift_register_ == Storage::Encodings::MFM::FMIDAddressMark) break; + } + if(index_count_ >= 2) return nullptr; + } + + crc_generator_.reset(); + sector->track = get_next_byte(); + sector->side = get_next_byte(); + sector->sector = get_next_byte(); + uint8_t size = get_next_byte(); + uint16_t header_crc = crc_generator_.get_value(); + if((header_crc >> 8) != get_next_byte()) continue; + if((header_crc & 0xff) != get_next_byte()) continue; + + // look for data mark + while(1) + { + run_for_cycles(1); + if(is_mfm_) + { + if(shift_register_ == Storage::Encodings::MFM::MFMAddressMark) + { + uint8_t mark = get_next_byte(); + if(mark == Storage::Encodings::MFM::MFMDataAddressByte) break; + if(mark == Storage::Encodings::MFM::MFMIDAddressByte) return nullptr; + } + } + else + { + if(shift_register_ == Storage::Encodings::MFM::FMDataAddressMark) break; + if(shift_register_ == Storage::Encodings::MFM::FMIDAddressMark) return nullptr; + } + if(index_count_ >= 2) return nullptr; + } + + size_t data_size = (size_t)(128 << size); + sector->data.reserve(data_size); + crc_generator_.reset(); + for(size_t c = 0; c < data_size; c++) + { + sector->data.push_back(get_next_byte()); + } + uint16_t data_crc = crc_generator_.get_value(); + if((data_crc >> 8) != get_next_byte()) continue; + if((data_crc & 0xff) != get_next_byte()) continue; + + return sector; + } + + return nullptr; +} + +std::shared_ptr Parser::get_sector(uint8_t sector) +{ + std::shared_ptr first_sector = get_next_sector(); + if(!first_sector) return first_sector; + if(first_sector->sector == sector) return first_sector; + + while(1) + { + std::shared_ptr next_sector = get_next_sector(); + if(next_sector->sector == first_sector->sector) return nullptr; + if(next_sector->sector == sector) return next_sector; + } +} diff --git a/Storage/Disk/Encodings/MFM.hpp b/Storage/Disk/Encodings/MFM.hpp index 57e4b11da..6fdccdd96 100644 --- a/Storage/Disk/Encodings/MFM.hpp +++ b/Storage/Disk/Encodings/MFM.hpp @@ -12,6 +12,8 @@ #include #include #include "../Disk.hpp" +#include "../DiskController.hpp" +#include "../../../NumberTheory/CRC.hpp" namespace Storage { namespace Encodings { @@ -56,6 +58,34 @@ class Encoder { std::unique_ptr GetMFMEncoder(std::vector &target); std::unique_ptr GetFMEncoder(std::vector &target); +class Parser: public Storage::Disk::Controller { + public: + Parser(bool is_mfm, const std::shared_ptr &disk); + + /*! + Attempts to read the sector located at @c track and @c sector. + + @returns a sector if one was found; @c nullptr otherwise. + */ + std::shared_ptr get_sector(uint8_t track, uint8_t sector); + + private: + std::shared_ptr drive; + unsigned int shift_register_; + int index_count_; + uint8_t track_; + int bit_count_; + NumberTheory::CRC16 crc_generator_; + bool is_mfm_; + + void process_input_bit(int value, unsigned int cycles_since_index_hole); + void process_index_hole(); + uint8_t get_next_byte(); + std::shared_ptr get_next_sector(); + std::shared_ptr get_sector(uint8_t sector); +}; + + } } } From e2b829f68e36c51235230553fb37d81953999441 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 25 Dec 2016 20:15:07 -0500 Subject: [PATCH 10/52] Made an attempt to write the proper address mark. --- Components/1770/1770.cpp | 20 ++++++++++++++++++++ Components/1770/1770.hpp | 1 + 2 files changed, 21 insertions(+) diff --git a/Components/1770/1770.cpp b/Components/1770/1770.cpp index c0e2a1bca..72956a8b9 100644 --- a/Components/1770/1770.cpp +++ b/Components/1770/1770.cpp @@ -624,6 +624,18 @@ void WD1770::posit_event(Event new_event_type) { write_byte(0); } + WAIT_FOR_EVENT(Event::DataWritten); + + if(is_double_density_) + { + write_raw_short(Storage::Encodings::MFM::MFMAddressMark); + write_byte(command_&1 ? Storage::Encodings::MFM::MFMDataAddressByte : Storage::Encodings::MFM::MFMDeletedDataAddressByte); + } + else + { + write_raw_short(command_&1 ? Storage::Encodings::MFM::FMDeletedDataAddressMark : Storage::Encodings::MFM::FMDataAddressMark); + } + WAIT_FOR_EVENT(Event::DataWritten); distance_into_section_ = 0; @@ -716,3 +728,11 @@ void WD1770::write_byte(uint8_t byte) { for(int c = 0; c < 8; c++) write_bit((byte << c)&0x80); } + +void WD1770::write_raw_short(uint16_t value) +{ + for(int c = 0; c < 16; c++) + { + Controller::write_bit(!!((value << c)&0x8000)); + } +} diff --git a/Components/1770/1770.hpp b/Components/1770/1770.hpp index 3aaefef88..a91796828 100644 --- a/Components/1770/1770.hpp +++ b/Components/1770/1770.hpp @@ -124,6 +124,7 @@ class WD1770: public Storage::Disk::Controller { int last_bit_; void write_bit(int bit); void write_byte(uint8_t byte); + void write_raw_short(uint16_t value); // ID buffer uint8_t header_[6]; From c0a1264ab040a59b5df7d5a76016cd9a6e138b65 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 25 Dec 2016 20:19:47 -0500 Subject: [PATCH 11/52] Slightly improved legibility. --- Components/1770/1770.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Components/1770/1770.cpp b/Components/1770/1770.cpp index 72956a8b9..8325f6904 100644 --- a/Components/1770/1770.cpp +++ b/Components/1770/1770.cpp @@ -629,11 +629,11 @@ void WD1770::posit_event(Event new_event_type) if(is_double_density_) { write_raw_short(Storage::Encodings::MFM::MFMAddressMark); - write_byte(command_&1 ? Storage::Encodings::MFM::MFMDataAddressByte : Storage::Encodings::MFM::MFMDeletedDataAddressByte); + write_byte((command_&0x01) ? Storage::Encodings::MFM::MFMDataAddressByte : Storage::Encodings::MFM::MFMDeletedDataAddressByte); } else { - write_raw_short(command_&1 ? Storage::Encodings::MFM::FMDeletedDataAddressMark : Storage::Encodings::MFM::FMDataAddressMark); + write_raw_short((command_&0x01) ? Storage::Encodings::MFM::FMDeletedDataAddressMark : Storage::Encodings::MFM::FMDataAddressMark); } WAIT_FOR_EVENT(Event::DataWritten); From acc35885cd0b013ffb6be03493bc32949f23d5ab Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 25 Dec 2016 20:38:25 -0500 Subject: [PATCH 12/52] Attempted to reduce track invalidations. --- Components/1770/1770.cpp | 2 ++ Storage/Disk/DiskController.cpp | 7 +++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/Components/1770/1770.cpp b/Components/1770/1770.cpp index 8325f6904..d465c3578 100644 --- a/Components/1770/1770.cpp +++ b/Components/1770/1770.cpp @@ -535,10 +535,12 @@ void WD1770::posit_event(Event new_event_type) } if(distance_into_section_ == 7) { + printf("Considering %d/%d\n", header_[0], header_[2]); is_reading_data_ = false; if(header_[0] == track_ && header_[2] == sector_ && (has_motor_on_line() || !(command_&0x02) || ((command_&0x08) >> 3) == header_[1])) { + printf("Found %d/%d\n", header_[0], header_[2]); // TODO: test CRC goto type2_read_or_write_data; } diff --git a/Storage/Disk/DiskController.cpp b/Storage/Disk/DiskController.cpp index 3aa3fa2cf..21cc62630 100644 --- a/Storage/Disk/DiskController.cpp +++ b/Storage/Disk/DiskController.cpp @@ -235,8 +235,11 @@ bool Controller::get_motor_on() void Controller::set_drive(std::shared_ptr drive) { - invalidate_track(); - drive_ = drive; + if(drive_ != drive) + { + invalidate_track(); + drive_ = drive; + } } void Controller::invalidate_track() From 26710c988d012548712f96ca1a9c10278d1905d0 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 25 Dec 2016 20:40:06 -0500 Subject: [PATCH 13/52] Modified SSD to ensure a fully-formatted surface is represented even if no track data is in the source file. This corrects the controller's sense of write success. --- Storage/Disk/Formats/SSD.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Storage/Disk/Formats/SSD.cpp b/Storage/Disk/Formats/SSD.cpp index 3eea47728..efb10b73b 100644 --- a/Storage/Disk/Formats/SSD.cpp +++ b/Storage/Disk/Formats/SSD.cpp @@ -63,8 +63,8 @@ std::shared_ptr SSD::get_uncached_track_at_position(unsigned int head, un new_sector.data.resize(256); fread(&new_sector.data[0], 1, 256, file_); - if(feof(file_)) - break; +// if(feof(file_)) +// new_sector.data[0]; sectors.push_back(std::move(new_sector)); } From 4fca30b81f2e4b0133b2f7d9691aa7cca28f258f Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 25 Dec 2016 21:06:58 -0500 Subject: [PATCH 14/52] Made the Plus 3 less chatty, documented `invalidate_track`. --- Machines/Electron/Plus3.cpp | 26 ++++++++++++++++++-------- Machines/Electron/Plus3.hpp | 1 + Storage/Disk/DiskController.hpp | 4 ++++ 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/Machines/Electron/Plus3.cpp b/Machines/Electron/Plus3.cpp index 52035dd3d..9100a860c 100644 --- a/Machines/Electron/Plus3.cpp +++ b/Machines/Electron/Plus3.cpp @@ -24,18 +24,28 @@ void Plus3::set_disk(std::shared_ptr disk, int drive) void Plus3::set_control_register(uint8_t control) { - // TODO: // bit 0 => enable or disable drive 1 // bit 1 => enable or disable drive 2 // bit 2 => side select // bit 3 => single density select - switch(control&3) + + uint8_t changes = control ^ last_control_; + last_control_ = control; + + if(changes&3) { - case 0: selected_drive_ = -1; set_drive(nullptr); break; - default: selected_drive_ = 0; set_drive(drives_[0]); break; - case 2: selected_drive_ = 1; set_drive(drives_[1]); break; + switch(control&3) + { + case 0: selected_drive_ = -1; set_drive(nullptr); break; + default: selected_drive_ = 0; set_drive(drives_[0]); break; + case 2: selected_drive_ = 1; set_drive(drives_[1]); break; + } } - if(drives_[0]) drives_[0]->set_head((control & 0x04) ? 1 : 0); - if(drives_[1]) drives_[1]->set_head((control & 0x04) ? 1 : 0); - set_is_double_density(!(control & 0x08)); + if(changes & 0x04) + { + invalidate_track(); + if(drives_[0]) drives_[0]->set_head((control & 0x04) ? 1 : 0); + if(drives_[1]) drives_[1]->set_head((control & 0x04) ? 1 : 0); + } + if(changes & 0x08) set_is_double_density(!(control & 0x08)); } diff --git a/Machines/Electron/Plus3.hpp b/Machines/Electron/Plus3.hpp index cd04b3877..8e6b7c0e4 100644 --- a/Machines/Electron/Plus3.hpp +++ b/Machines/Electron/Plus3.hpp @@ -23,6 +23,7 @@ class Plus3 : public WD::WD1770 { private: std::shared_ptr drives_[2]; int selected_drive_; + uint8_t last_control_; }; } diff --git a/Storage/Disk/DiskController.hpp b/Storage/Disk/DiskController.hpp index f309307bc..a39b74b48 100644 --- a/Storage/Disk/DiskController.hpp +++ b/Storage/Disk/DiskController.hpp @@ -49,6 +49,10 @@ class Controller: public DigitalPhaseLockedLoop::Delegate, public TimedEventLoop Sets the current drive. */ void set_drive(std::shared_ptr drive); + + /*! + Announces that the track the drive sees is about to change for a reason unknownt to the controller. + */ void invalidate_track(); /*! From a6d038cad9b191f76b1d9ee7ac5aa50f3df92d1b Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 25 Dec 2016 21:32:14 -0500 Subject: [PATCH 15/52] Eliminated special case that doesn't seek properly and isn't needed. Added TODO. --- Storage/Disk/DiskController.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/Storage/Disk/DiskController.cpp b/Storage/Disk/DiskController.cpp index 21cc62630..5c61311fe 100644 --- a/Storage/Disk/DiskController.cpp +++ b/Storage/Disk/DiskController.cpp @@ -34,15 +34,11 @@ void Controller::setup_track() Time offset; Time track_time_now = get_time_into_track(); - if(track_ && track_time_now > Time(0)) + if(track_) { Time time_found = track_->seek_to(track_time_now); offset = track_time_now - time_found; } - else - { - offset = track_time_now; - } get_next_event(offset); } @@ -170,6 +166,8 @@ void Controller::end_writing() if(!patched_track_) { + // TODO: is the track already actually a patched track? + // see dynamic_pointer_cast patched_track_.reset(new PCMPatchedTrack(track_)); } patched_track_->add_segment(write_start_time_, write_segment_); From b538ee5bd8bfb3eb21c491a35afb1de07f78b286 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 25 Dec 2016 21:32:50 -0500 Subject: [PATCH 16/52] Fixed discovery of correct active period and setting of track time, when seeking. --- Storage/Disk/PCMPatchedTrack.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Storage/Disk/PCMPatchedTrack.cpp b/Storage/Disk/PCMPatchedTrack.cpp index c25b00f3b..71ef0de86 100644 --- a/Storage/Disk/PCMPatchedTrack.cpp +++ b/Storage/Disk/PCMPatchedTrack.cpp @@ -196,15 +196,16 @@ Track::Event PCMPatchedTrack::get_next_event() Storage::Time PCMPatchedTrack::seek_to(const Time &time_since_index_hole) { - // start at the beginning and continue while segments start after the time sought + // start at the beginning and continue while segments end before reaching the time sought active_period_ = periods_.begin(); - while(active_period_->start_time > time_since_index_hole) active_period_++; + while(active_period_->end_time < time_since_index_hole) active_period_++; // allow whatever storage represents the period found to perform its seek if(active_period_->event_source) - return active_period_->event_source->seek_to(time_since_index_hole - active_period_->start_time) + active_period_->start_time; + current_time_ = active_period_->event_source->seek_to(time_since_index_hole - active_period_->start_time) + active_period_->start_time; else - return underlying_track_->seek_to(time_since_index_hole); + current_time_ = underlying_track_->seek_to(time_since_index_hole); + return current_time_; } void PCMPatchedTrack::Period::push_start_to_time(const Storage::Time &new_start_time) From 742c5df36765ba1d09696af9c77dd14d2e6e028a Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 25 Dec 2016 22:00:39 -0500 Subject: [PATCH 17/52] With lots of logging arising temporarily, fixed bug whereby conversion to a patched track would lead to holding a track with a distinct measure of time, leading to improperly-placed patches. --- Components/1770/1770.cpp | 10 +++++++--- Storage/Disk/DiskController.cpp | 10 +++++++--- Storage/Disk/DiskController.hpp | 2 +- Storage/Disk/PCMPatchedTrack.cpp | 6 +++--- Storage/Disk/PCMPatchedTrack.hpp | 2 +- 5 files changed, 19 insertions(+), 11 deletions(-) diff --git a/Components/1770/1770.cpp b/Components/1770/1770.cpp index d465c3578..a725a4770 100644 --- a/Components/1770/1770.cpp +++ b/Components/1770/1770.cpp @@ -535,7 +535,7 @@ void WD1770::posit_event(Event new_event_type) } if(distance_into_section_ == 7) { - printf("Considering %d/%d\n", header_[0], header_[2]); + printf("Considering %d/%d at %0.4f\n", header_[0], header_[2], get_time_into_track().get_float()); is_reading_data_ = false; if(header_[0] == track_ && header_[2] == sector_ && (has_motor_on_line() || !(command_&0x02) || ((command_&0x08) >> 3) == header_[1])) @@ -563,6 +563,7 @@ void WD1770::posit_event(Event new_event_type) }); distance_into_section_ = 0; is_reading_data_ = true; + printf("\n"); goto type2_read_byte; } goto type2_read_data; @@ -570,7 +571,7 @@ void WD1770::posit_event(Event new_event_type) type2_read_byte: WAIT_FOR_EVENT(Event::Token); if(latest_token_.type != Token::Byte) goto type2_read_byte; - data_ = latest_token_.byte_value; + data_ = latest_token_.byte_value; printf("%02x", data_); update_status([] (Status &status) { status.lost_data |= status.data_request; status.data_request = true; @@ -579,6 +580,7 @@ void WD1770::posit_event(Event new_event_type) if(distance_into_section_ == 128 << header_[3]) { distance_into_section_ = 0; + printf("\n"); goto type2_check_crc; } goto type2_read_byte; @@ -640,9 +642,10 @@ void WD1770::posit_event(Event new_event_type) WAIT_FOR_EVENT(Event::DataWritten); distance_into_section_ = 0; + printf("\n"); type2_write_loop: - write_byte(data_); + write_byte(data_); printf("%02x", data_); update_status([] (Status &status) { status.data_request = true; }); @@ -650,6 +653,7 @@ void WD1770::posit_event(Event new_event_type) distance_into_section_++; if(distance_into_section_ == 128 << header_[3]) { + printf("\n"); goto type2_write_crc; } diff --git a/Storage/Disk/DiskController.cpp b/Storage/Disk/DiskController.cpp index 5c61311fe..47c971a22 100644 --- a/Storage/Disk/DiskController.cpp +++ b/Storage/Disk/DiskController.cpp @@ -166,11 +166,15 @@ void Controller::end_writing() if(!patched_track_) { - // TODO: is the track already actually a patched track? - // see dynamic_pointer_cast - patched_track_.reset(new PCMPatchedTrack(track_)); + // Avoid creating a new patched track if this one is already patched + patched_track_ = std::dynamic_pointer_cast(track_); + if(!patched_track_) + { + patched_track_.reset(new PCMPatchedTrack(track_)); + } } patched_track_->add_segment(write_start_time_, write_segment_); + invalidate_track(); // TEMPORARY: to force a seek } #pragma mark - PLL control and delegate diff --git a/Storage/Disk/DiskController.hpp b/Storage/Disk/DiskController.hpp index a39b74b48..19e9e8755 100644 --- a/Storage/Disk/DiskController.hpp +++ b/Storage/Disk/DiskController.hpp @@ -110,6 +110,7 @@ class Controller: public DigitalPhaseLockedLoop::Delegate, public TimedEventLoop void step(int direction); virtual bool get_drive_is_ready(); bool get_drive_is_read_only(); + Time get_time_into_track(); private: Time bit_length_; @@ -135,7 +136,6 @@ class Controller: public DigitalPhaseLockedLoop::Delegate, public TimedEventLoop Time cycles_per_bit_; void setup_track(); - Time get_time_into_track(); }; } diff --git a/Storage/Disk/PCMPatchedTrack.cpp b/Storage/Disk/PCMPatchedTrack.cpp index 71ef0de86..a70ec9da9 100644 --- a/Storage/Disk/PCMPatchedTrack.cpp +++ b/Storage/Disk/PCMPatchedTrack.cpp @@ -43,8 +43,7 @@ void PCMPatchedTrack::add_segment(const Time &start_time, const PCMSegment &segm // the vector may have been resized, potentially invalidating active_period_ even if // the thing it pointed to is still the same thing. So work it out afresh. - active_period_ = periods_.begin(); - while(active_period_->start_time > current_time_) active_period_++; + insertion_error_ = current_time_ - seek_to(current_time_); } void PCMPatchedTrack::insert_period(const Period &period) @@ -155,7 +154,8 @@ Track::Event PCMPatchedTrack::get_next_event() else event = underlying_track_->get_next_event(); // see what time that gets us to. If it's still within the current period, return the found event - Time event_time = current_time_ + event.length - period_error; + Time event_time = current_time_ + event.length - period_error - insertion_error_; + insertion_error_.set_zero(); if(event_time < active_period_->end_time) { current_time_ = event_time; diff --git a/Storage/Disk/PCMPatchedTrack.hpp b/Storage/Disk/PCMPatchedTrack.hpp index 83182b39b..2ea8ffd04 100644 --- a/Storage/Disk/PCMPatchedTrack.hpp +++ b/Storage/Disk/PCMPatchedTrack.hpp @@ -52,7 +52,7 @@ class PCMPatchedTrack: public Track { }; std::vector periods_; std::vector::iterator active_period_; - Time current_time_; + Time current_time_, insertion_error_; void insert_period(const Period &period); }; From 83c433c1421cefc538ab16605de412ead2c87f31 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 26 Dec 2016 12:48:49 -0500 Subject: [PATCH 18/52] Deviated from the data sheet, which seems likely to be correct. Hence removed a whole load of the temporary logging. --- Components/1770/1770.cpp | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/Components/1770/1770.cpp b/Components/1770/1770.cpp index a725a4770..e320d01f4 100644 --- a/Components/1770/1770.cpp +++ b/Components/1770/1770.cpp @@ -563,7 +563,6 @@ void WD1770::posit_event(Event new_event_type) }); distance_into_section_ = 0; is_reading_data_ = true; - printf("\n"); goto type2_read_byte; } goto type2_read_data; @@ -571,7 +570,7 @@ void WD1770::posit_event(Event new_event_type) type2_read_byte: WAIT_FOR_EVENT(Event::Token); if(latest_token_.type != Token::Byte) goto type2_read_byte; - data_ = latest_token_.byte_value; printf("%02x", data_); + data_ = latest_token_.byte_value; update_status([] (Status &status) { status.lost_data |= status.data_request; status.data_request = true; @@ -580,7 +579,6 @@ void WD1770::posit_event(Event new_event_type) if(distance_into_section_ == 128 << header_[3]) { distance_into_section_ = 0; - printf("\n"); goto type2_check_crc; } goto type2_read_byte; @@ -633,7 +631,7 @@ void WD1770::posit_event(Event new_event_type) if(is_double_density_) { write_raw_short(Storage::Encodings::MFM::MFMAddressMark); - write_byte((command_&0x01) ? Storage::Encodings::MFM::MFMDataAddressByte : Storage::Encodings::MFM::MFMDeletedDataAddressByte); + write_byte((command_&0x01) ? Storage::Encodings::MFM::MFMDeletedDataAddressByte : Storage::Encodings::MFM::MFMDataAddressByte); } else { @@ -642,20 +640,25 @@ void WD1770::posit_event(Event new_event_type) WAIT_FOR_EVENT(Event::DataWritten); distance_into_section_ = 0; - printf("\n"); type2_write_loop: - write_byte(data_); printf("%02x", data_); - update_status([] (Status &status) { - status.data_request = true; - }); - WAIT_FOR_EVENT(Event::DataWritten); + /* + This deviates from the data sheet slightly since that would prima facie request one more byte + of data than is actually written — the last time around the loop it has transferred from the + data register to the data shift register, set data request, written the byte, checked that data + request has been satified, then finally considers whether all bytes are done. Based on both + natural expectations and the way that emulated machines responded, I believe that to be a + documentation error. + */ + write_byte(data_); distance_into_section_++; - if(distance_into_section_ == 128 << header_[3]) + if(distance_into_section_ < 128 << header_[3]) { - printf("\n"); - goto type2_write_crc; + update_status([] (Status &status) { + status.data_request = true; + }); } + WAIT_FOR_EVENT(Event::DataWritten); if(status_.data_request) { @@ -665,6 +668,10 @@ void WD1770::posit_event(Event new_event_type) goto wait_for_command; } + if(distance_into_section_ == 128 << header_[3]) + { + goto type2_write_crc; + } goto type2_write_loop; type2_write_crc: From 0490a4705893a62e32f34b08930996b6b273d820 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 26 Dec 2016 14:24:33 -0500 Subject: [PATCH 19/52] Worked on the all-around framework for decoding sectors back from tracks when closing down a file. Hit the wall that the parser is more observant of CRCs than the WD. No, really. So I guess I have to stop avoiding that whole issue. --- Components/1770/1770.cpp | 2 +- Storage/Disk/Disk.hpp | 1 + Storage/Disk/DiskController.hpp | 2 +- Storage/Disk/Drive.cpp | 13 +++++++++++-- Storage/Disk/Drive.hpp | 10 ++++++++-- Storage/Disk/Encodings/MFM.cpp | 15 +++++++++++++-- Storage/Disk/Encodings/MFM.hpp | 3 +++ Storage/Disk/Formats/SSD.cpp | 23 +++++++++++++++++++++++ Storage/Disk/Formats/SSD.hpp | 1 + 9 files changed, 62 insertions(+), 8 deletions(-) diff --git a/Components/1770/1770.cpp b/Components/1770/1770.cpp index e320d01f4..ee0763b40 100644 --- a/Components/1770/1770.cpp +++ b/Components/1770/1770.cpp @@ -535,7 +535,7 @@ void WD1770::posit_event(Event new_event_type) } if(distance_into_section_ == 7) { - printf("Considering %d/%d at %0.4f\n", header_[0], header_[2], get_time_into_track().get_float()); + printf("Considering %d/%d\n", header_[0], header_[2]); is_reading_data_ = false; if(header_[0] == track_ && header_[2] == sector_ && (has_motor_on_line() || !(command_&0x02) || ((command_&0x08) >> 3) == header_[1])) diff --git a/Storage/Disk/Disk.hpp b/Storage/Disk/Disk.hpp index 514bc0c61..7b435d838 100644 --- a/Storage/Disk/Disk.hpp +++ b/Storage/Disk/Disk.hpp @@ -66,6 +66,7 @@ class Track { */ class Disk { public: + virtual ~Disk() {} /*! @returns the number of discrete positions that this disk uses to model its complete surface area. diff --git a/Storage/Disk/DiskController.hpp b/Storage/Disk/DiskController.hpp index 19e9e8755..a39b74b48 100644 --- a/Storage/Disk/DiskController.hpp +++ b/Storage/Disk/DiskController.hpp @@ -110,7 +110,6 @@ class Controller: public DigitalPhaseLockedLoop::Delegate, public TimedEventLoop void step(int direction); virtual bool get_drive_is_ready(); bool get_drive_is_read_only(); - Time get_time_into_track(); private: Time bit_length_; @@ -136,6 +135,7 @@ class Controller: public DigitalPhaseLockedLoop::Delegate, public TimedEventLoop Time cycles_per_bit_; void setup_track(); + Time get_time_into_track(); }; } diff --git a/Storage/Disk/Drive.cpp b/Storage/Disk/Drive.cpp index 0b9fb34cb..3b58d8fbb 100644 --- a/Storage/Disk/Drive.cpp +++ b/Storage/Disk/Drive.cpp @@ -14,14 +14,21 @@ using namespace Storage::Disk; Drive::Drive() : head_position_(0), head_(0) {} -void Drive::set_disk(std::shared_ptr disk) +void Drive::set_disk(const std::shared_ptr &disk) { disk_ = disk; + track_ = nullptr; +} + +void Drive::set_disk_with_track(const std::shared_ptr &track) +{ + disk_ = nullptr; + track_ = track; } bool Drive::has_disk() { - return (bool)disk_; + return (bool)disk_ || (bool)track_; } bool Drive::get_is_track_zero() @@ -42,12 +49,14 @@ void Drive::set_head(unsigned int head) bool Drive::get_is_read_only() { if(disk_) return disk_->get_is_read_only(); + if(track_) return true; return false; } std::shared_ptr Drive::get_track() { if(disk_) return disk_->get_track_at_position(head_, (unsigned int)head_position_); + if(track_) return track_; return nullptr; } diff --git a/Storage/Disk/Drive.hpp b/Storage/Disk/Drive.hpp index 1506e8a2c..82e34fcd6 100644 --- a/Storage/Disk/Drive.hpp +++ b/Storage/Disk/Drive.hpp @@ -20,9 +20,14 @@ class Drive { Drive(); /*! - Inserts @c disk into the drive. + Replaces whatever is in the drive with @c disk. */ - void set_disk(std::shared_ptr disk); + void set_disk(const std::shared_ptr &disk); + + /*! + Replaces whatever is in the drive with a disk that contains endless copies of @c track. + */ + void set_disk_with_track(const std::shared_ptr &track); /*! @returns @c true if a disk is currently inserted; @c false otherwise. @@ -61,6 +66,7 @@ class Drive { void set_track(const std::shared_ptr &track); private: + std::shared_ptr track_; std::shared_ptr disk_; int head_position_; unsigned int head_; diff --git a/Storage/Disk/Encodings/MFM.cpp b/Storage/Disk/Encodings/MFM.cpp index 2af4e5bfb..69767ad20 100644 --- a/Storage/Disk/Encodings/MFM.cpp +++ b/Storage/Disk/Encodings/MFM.cpp @@ -214,7 +214,7 @@ std::unique_ptr Storage::Encodings::MFM::GetFMEncoder(std::vector &disk) : +Parser::Parser(bool is_mfm) : Storage::Disk::Controller(4000000, 1, 300), crc_generator_(0x1021, 0xffff), shift_register_(0), track_(0), is_mfm_(is_mfm) @@ -225,11 +225,22 @@ Parser::Parser(bool is_mfm, const std::shared_ptr &disk) : set_expected_bit_length(bit_length); drive.reset(new Storage::Disk::Drive); - drive->set_disk(disk); set_drive(drive); set_motor_on(true); } +Parser::Parser(bool is_mfm, const std::shared_ptr &disk) : + Parser(is_mfm) +{ + drive->set_disk(disk); +} + +Parser::Parser(bool is_mfm, const std::shared_ptr &track) : + Parser(is_mfm) +{ + drive->set_disk_with_track(track); +} + std::shared_ptr Parser::get_sector(uint8_t track, uint8_t sector) { int difference = (int)track - (int)track_; diff --git a/Storage/Disk/Encodings/MFM.hpp b/Storage/Disk/Encodings/MFM.hpp index 6fdccdd96..5f26ec7c0 100644 --- a/Storage/Disk/Encodings/MFM.hpp +++ b/Storage/Disk/Encodings/MFM.hpp @@ -61,6 +61,7 @@ std::unique_ptr GetFMEncoder(std::vector &target); class Parser: public Storage::Disk::Controller { public: Parser(bool is_mfm, const std::shared_ptr &disk); + Parser(bool is_mfm, const std::shared_ptr &track); /*! Attempts to read the sector located at @c track and @c sector. @@ -70,6 +71,8 @@ class Parser: public Storage::Disk::Controller { std::shared_ptr get_sector(uint8_t track, uint8_t sector); private: + Parser(bool is_mfm); + std::shared_ptr drive; unsigned int shift_register_; int index_count_; diff --git a/Storage/Disk/Formats/SSD.cpp b/Storage/Disk/Formats/SSD.cpp index efb10b73b..4f6863214 100644 --- a/Storage/Disk/Formats/SSD.cpp +++ b/Storage/Disk/Formats/SSD.cpp @@ -30,6 +30,29 @@ SSD::SSD(const char *file_name) : else if(track_count_ < 80) track_count_ = 80; } +SSD::~SSD() +{ + if(get_is_modified()) + { + for(unsigned int head = 0; head < head_count_; head++) + { + for(unsigned int track = 0; track < track_count_; track++) + { + std::shared_ptr modified_track = get_modified_track_at_position(head, track); + if(modified_track) + { + Storage::Encodings::MFM::Parser parser(false, modified_track); + for(unsigned int c = 0; c < 10; c++) + { + std::shared_ptr sector = parser.get_sector((uint8_t)track, (uint8_t)c); + printf("Sector %d: %p\n", c, sector.get()); + } + } + } + } + } +} + unsigned int SSD::get_head_position_count() { return track_count_; diff --git a/Storage/Disk/Formats/SSD.hpp b/Storage/Disk/Formats/SSD.hpp index 90febd2cb..b7c21ccb3 100644 --- a/Storage/Disk/Formats/SSD.hpp +++ b/Storage/Disk/Formats/SSD.hpp @@ -27,6 +27,7 @@ class SSD: public Disk, public Storage::FileHolder { @throws ErrorNotSSD if the file doesn't appear to contain a .SSD format image. */ SSD(const char *file_name); + ~SSD(); enum { ErrorNotSSD, From 9c0f622a2eea5de371ff3ec81c8ffb3dcca4a716 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 26 Dec 2016 16:46:26 -0500 Subject: [PATCH 20/52] Started working CRC checking into the 1770. Discovered immediately that my generated CRC does not match that built into the Oric disk images. So mine is pretty-much certainly wrong. An opportunity for learning! --- Components/1770/1770.cpp | 20 ++++++++++++++++---- Components/1770/1770.hpp | 4 ++++ 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/Components/1770/1770.cpp b/Components/1770/1770.cpp index ee0763b40..6c4c5c728 100644 --- a/Components/1770/1770.cpp +++ b/Components/1770/1770.cpp @@ -29,6 +29,7 @@ WD1770::Status::Status() : WD1770::WD1770(Personality p) : Storage::Disk::Controller(8000000, 16, 300), + crc_generator_(0x1021, 0xffff), interesting_event_mask_(Event::Command), resume_point_(0), delay_time_(0), @@ -563,6 +564,7 @@ void WD1770::posit_event(Event new_event_type) }); distance_into_section_ = 0; is_reading_data_ = true; + crc_generator_.reset(); goto type2_read_byte; } goto type2_read_data; @@ -571,6 +573,7 @@ void WD1770::posit_event(Event new_event_type) WAIT_FOR_EVENT(Event::Token); if(latest_token_.type != Token::Byte) goto type2_read_byte; data_ = latest_token_.byte_value; + crc_generator_.add(data_); update_status([] (Status &status) { status.lost_data |= status.data_request; status.data_request = true; @@ -590,7 +593,12 @@ void WD1770::posit_event(Event new_event_type) distance_into_section_++; if(distance_into_section_ == 2) { - // TODO: check CRC + uint16_t crc = crc_generator_.get_value(); + if((crc >> 8) != header_[0] || (crc&0xff) != header_[1]) + { + printf("CRC error: %04x v %02x%02x\n", crc, header_[0], header_[1]); + } + if(command_ & 0x10) { sector_++; @@ -640,6 +648,7 @@ void WD1770::posit_event(Event new_event_type) WAIT_FOR_EVENT(Event::DataWritten); distance_into_section_ = 0; + crc_generator_.reset(); type2_write_loop: /* @@ -650,6 +659,7 @@ void WD1770::posit_event(Event new_event_type) natural expectations and the way that emulated machines responded, I believe that to be a documentation error. */ + crc_generator_.add(data_); write_byte(data_); distance_into_section_++; if(distance_into_section_ < 128 << header_[3]) @@ -675,9 +685,11 @@ void WD1770::posit_event(Event new_event_type) goto type2_write_loop; type2_write_crc: - // TODO: write CRC - write_byte(0); - write_byte(0); + { + uint16_t crc = crc_generator_.get_value(); + write_byte(crc >> 8); + write_byte((crc & 0xff)); + } write_byte(0xff); WAIT_FOR_EVENT(Event::DataWritten); end_writing(); diff --git a/Components/1770/1770.hpp b/Components/1770/1770.hpp index a91796828..92211dbde 100644 --- a/Components/1770/1770.hpp +++ b/Components/1770/1770.hpp @@ -10,6 +10,7 @@ #define _770_hpp #include "../../Storage/Disk/DiskController.hpp" +#include "../../NumberTheory/CRC.hpp" namespace WD { @@ -129,6 +130,9 @@ class WD1770: public Storage::Disk::Controller { // ID buffer uint8_t header_[6]; + // CRC generator + NumberTheory::CRC16 crc_generator_; + // 1793 head-loading logic bool head_is_loaded_; From 99993a1b24659c64650101632bac6b1750b6ffa5 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 27 Dec 2016 19:03:46 -0500 Subject: [PATCH 21/52] Since it's about to become important that objective results match, added a couple of objective-result tests for the CRC generator. --- NumberTheory/CRC.hpp | 2 +- .../Clock Signal.xcodeproj/project.pbxproj | 4 ++ OSBindings/Mac/Clock SignalTests/CRCTests.mm | 71 +++++++++++++++++++ 3 files changed, 76 insertions(+), 1 deletion(-) create mode 100644 OSBindings/Mac/Clock SignalTests/CRCTests.mm diff --git a/NumberTheory/CRC.hpp b/NumberTheory/CRC.hpp index a705e3d4c..007b63096 100644 --- a/NumberTheory/CRC.hpp +++ b/NumberTheory/CRC.hpp @@ -28,7 +28,7 @@ class CRC16 { value_ = (uint16_t)(value_ << 1) ^ exclusive_or; } } - inline uint16_t get_value() { return value_; } + inline uint16_t get_value() const { return value_; } private: uint16_t reset_value_, polynomial_; diff --git a/OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj b/OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj index 631e4e8eb..64b8747f6 100644 --- a/OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj +++ b/OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj @@ -354,6 +354,7 @@ 4BB299F71B587D8400A49093 /* txan in Resources */ = {isa = PBXBuildFile; fileRef = 4BB298EB1B587D8400A49093 /* txan */; }; 4BB299F81B587D8400A49093 /* txsn in Resources */ = {isa = PBXBuildFile; fileRef = 4BB298EC1B587D8400A49093 /* txsn */; }; 4BB299F91B587D8400A49093 /* tyan in Resources */ = {isa = PBXBuildFile; fileRef = 4BB298ED1B587D8400A49093 /* tyan */; }; + 4BB2A9AF1E13367E001A5C23 /* CRCTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 4BB2A9AE1E13367E001A5C23 /* CRCTests.mm */; }; 4BB697CB1D4B6D3E00248BDF /* TimedEventLoop.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4BB697C91D4B6D3E00248BDF /* TimedEventLoop.cpp */; }; 4BB697CE1D4BA44400248BDF /* CommodoreGCR.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4BB697CC1D4BA44400248BDF /* CommodoreGCR.cpp */; }; 4BB73EA21B587A5100552FC2 /* AppDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4BB73EA11B587A5100552FC2 /* AppDelegate.swift */; }; @@ -841,6 +842,7 @@ 4BB298EB1B587D8400A49093 /* txan */ = {isa = PBXFileReference; lastKnownFileType = file; path = txan; sourceTree = ""; }; 4BB298EC1B587D8400A49093 /* txsn */ = {isa = PBXFileReference; lastKnownFileType = file; path = txsn; sourceTree = ""; }; 4BB298ED1B587D8400A49093 /* tyan */ = {isa = PBXFileReference; lastKnownFileType = file; path = tyan; sourceTree = ""; }; + 4BB2A9AE1E13367E001A5C23 /* CRCTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = CRCTests.mm; sourceTree = ""; }; 4BB697C61D4B558F00248BDF /* Factors.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; name = Factors.hpp; path = ../../NumberTheory/Factors.hpp; sourceTree = ""; }; 4BB697C91D4B6D3E00248BDF /* TimedEventLoop.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = TimedEventLoop.cpp; sourceTree = ""; }; 4BB697CA1D4B6D3E00248BDF /* TimedEventLoop.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = TimedEventLoop.hpp; sourceTree = ""; }; @@ -1691,6 +1693,7 @@ isa = PBXGroup; children = ( 4B5073091DDFCFDF00C48FBD /* ArrayBuilderTests.mm */, + 4BB2A9AE1E13367E001A5C23 /* CRCTests.mm */, 4B121F941E05E66800BFDA12 /* PCMPatchedTrackTests.mm */, 4B121F9A1E06293F00BFDA12 /* PCMSegmentEventSourceTests.mm */, 4BD4A8CF1E077FD20020D856 /* PCMTrackTests.mm */, @@ -2465,6 +2468,7 @@ 4BEF6AAC1D35D1C400E73575 /* DPLLTests.swift in Sources */, 4B3BA0CF1D318B44005DD7A7 /* MOS6522Bridge.mm in Sources */, 4BC751B21D157E61006C31D9 /* 6522Tests.swift in Sources */, + 4BB2A9AF1E13367E001A5C23 /* CRCTests.mm in Sources */, 4B3BA0D01D318B44005DD7A7 /* MOS6532Bridge.mm in Sources */, 4B3BA0C31D318AEC005DD7A7 /* C1540Tests.swift in Sources */, 4B1414621B58888700E04248 /* KlausDormannTests.swift in Sources */, diff --git a/OSBindings/Mac/Clock SignalTests/CRCTests.mm b/OSBindings/Mac/Clock SignalTests/CRCTests.mm new file mode 100644 index 000000000..f5dc603b8 --- /dev/null +++ b/OSBindings/Mac/Clock SignalTests/CRCTests.mm @@ -0,0 +1,71 @@ +// +// CRCTests.m +// Clock Signal +// +// Created by Thomas Harte on 27/12/2016. +// Copyright © 2016 Thomas Harte. All rights reserved. +// + +#import +#include "CRC.hpp" + +@interface CRCTests : XCTestCase +@end + +@implementation CRCTests + +- (NumberTheory::CRC16)mfmCRCGenerator +{ + return NumberTheory::CRC16(0x1021, 0xffff); +} + +- (uint16_t)crcOfData:(uint8_t *)data length:(size_t)length generator:(NumberTheory::CRC16 &)generator +{ + generator.reset(); + for(size_t c = 0; c < length; c++) generator.add(data[c]); + return generator.get_value(); +} + +- (void)testIDMark +{ + uint8_t IDMark[] = + { + 0xa1, 0xa1, 0xa1, 0xfe, 0x00, 0x00, 0x01, 0x01 + }; + uint16_t crc = 0xfa0c; + NumberTheory::CRC16 crcGenerator = self.mfmCRCGenerator; + + uint16_t computedCRC = [self crcOfData:IDMark length:sizeof(IDMark) generator:crcGenerator]; + XCTAssert(computedCRC == crc, @"Calculated CRC should have been %04x, was %04x", crc, computedCRC); +} + +- (void)testData +{ + uint8_t sectorData[] = + { + 0xa1, 0xa1, 0xa1, 0xfb, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x20, 0x20, 0x20, 0x20, + 0x20, 0x20, 0x20, 0x20, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, 0x01, 0x00, 0x53, 0x45, 0x44, 0x4f, + 0x52, 0x49, 0x43, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, + 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, + 0x20, 0x20, 0x20, 0x20, 0x53, 0x45, 0x44, 0x4f, 0x52, 0x49, 0x43, 0x20, 0x56, 0x31, 0x2e, 0x30, + 0x30, 0x36, 0x20, 0x30, 0x31, 0x2f, 0x30, 0x31, 0x2f, 0x38, 0x36, 0x20, 0x20, 0x20, 0x20, 0x20, + 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, + 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, + 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, + 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, + 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, + 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, + 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, + 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, + 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, + 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, + 0x20, 0x20, 0x20, 0x20 + }; + uint16_t crc = 0x4de7; + NumberTheory::CRC16 crcGenerator = self.mfmCRCGenerator; + + uint16_t computedCRC = [self crcOfData:sectorData length:sizeof(sectorData) generator:crcGenerator]; + XCTAssert(computedCRC == crc, @"Calculated CRC should have been %04x, was %04x", crc, computedCRC); +} + +@end From a56817275811d5bfb986b2aa61b52453bf64d5a3 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 28 Dec 2016 18:29:37 -0500 Subject: [PATCH 22/52] =?UTF-8?q?Made=20steps=20towards=20proper=20CRC=20g?= =?UTF-8?q?eneration.=20Am=20currently=20comparing=20against=20Oric=20disk?= =?UTF-8?q?=20images,=20as=20=E2=80=94=20amongst=20other=20things=20?= =?UTF-8?q?=E2=80=94=20they=20include=20precomputed=20CRCs.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Components/1770/1770.cpp | 34 +++++++---- Machines/Oric/Microdisc.cpp | 59 ++++++++++++-------- Machines/Oric/Microdisc.hpp | 1 + NumberTheory/CRC.hpp | 1 + OSBindings/Mac/Clock SignalTests/CRCTests.mm | 3 +- Storage/Disk/Encodings/MFM.cpp | 57 +++++++++---------- Storage/Disk/Encodings/MFM.hpp | 9 ++- 7 files changed, 99 insertions(+), 65 deletions(-) diff --git a/Components/1770/1770.cpp b/Components/1770/1770.cpp index 6c4c5c728..7ea6a4b90 100644 --- a/Components/1770/1770.cpp +++ b/Components/1770/1770.cpp @@ -193,13 +193,14 @@ void WD1770::process_input_bit(int value, unsigned int cycles_since_index_hole) { switch(shift_register_ & 0xffff) { - case Storage::Encodings::MFM::MFMIndexAddressMark: + case Storage::Encodings::MFM::MFMIndexSync: bits_since_token_ = 0; is_awaiting_marker_value_ = true; return; - case Storage::Encodings::MFM::MFMAddressMark: + case Storage::Encodings::MFM::MFMSync: bits_since_token_ = 0; is_awaiting_marker_value_ = true; + crc_generator_.set_value(Storage::Encodings::MFM::MFMPostSyncCRCValue); return; default: break; @@ -250,6 +251,7 @@ void WD1770::process_input_bit(int value, unsigned int cycles_since_index_hole) } } + crc_generator_.add(latest_token_.byte_value); posit_event(Event::Token); return; } @@ -542,7 +544,17 @@ void WD1770::posit_event(Event new_event_type) (has_motor_on_line() || !(command_&0x02) || ((command_&0x08) >> 3) == header_[1])) { printf("Found %d/%d\n", header_[0], header_[2]); - // TODO: test CRC + if(crc_generator_.get_value()) + { + update_status([] (Status &status) { + status.crc_error = true; + }); + goto type2_get_header; + } + + update_status([] (Status &status) { + status.crc_error = false; + }); goto type2_read_or_write_data; } distance_into_section_ = 0; @@ -564,7 +576,6 @@ void WD1770::posit_event(Event new_event_type) }); distance_into_section_ = 0; is_reading_data_ = true; - crc_generator_.reset(); goto type2_read_byte; } goto type2_read_data; @@ -573,7 +584,6 @@ void WD1770::posit_event(Event new_event_type) WAIT_FOR_EVENT(Event::Token); if(latest_token_.type != Token::Byte) goto type2_read_byte; data_ = latest_token_.byte_value; - crc_generator_.add(data_); update_status([] (Status &status) { status.lost_data |= status.data_request; status.data_request = true; @@ -593,10 +603,12 @@ void WD1770::posit_event(Event new_event_type) distance_into_section_++; if(distance_into_section_ == 2) { - uint16_t crc = crc_generator_.get_value(); - if((crc >> 8) != header_[0] || (crc&0xff) != header_[1]) + if(crc_generator_.get_value()) { - printf("CRC error: %04x v %02x%02x\n", crc, header_[0], header_[1]); + update_status([this] (Status &status) { + status.crc_error = true; + }); + goto wait_for_command; } if(command_ & 0x10) @@ -638,17 +650,18 @@ void WD1770::posit_event(Event new_event_type) if(is_double_density_) { - write_raw_short(Storage::Encodings::MFM::MFMAddressMark); + crc_generator_.set_value(Storage::Encodings::MFM::MFMPostSyncCRCValue); write_byte((command_&0x01) ? Storage::Encodings::MFM::MFMDeletedDataAddressByte : Storage::Encodings::MFM::MFMDataAddressByte); } else { + crc_generator_.reset(); + crc_generator_.add((command_&0x01) ? Storage::Encodings::MFM::MFMDeletedDataAddressByte : Storage::Encodings::MFM::MFMDataAddressByte); write_raw_short((command_&0x01) ? Storage::Encodings::MFM::FMDeletedDataAddressMark : Storage::Encodings::MFM::FMDataAddressMark); } WAIT_FOR_EVENT(Event::DataWritten); distance_into_section_ = 0; - crc_generator_.reset(); type2_write_loop: /* @@ -752,6 +765,7 @@ void WD1770::write_bit(int bit) void WD1770::write_byte(uint8_t byte) { for(int c = 0; c < 8; c++) write_bit((byte << c)&0x80); + crc_generator_.add(byte); } void WD1770::write_raw_short(uint16_t value) diff --git a/Machines/Oric/Microdisc.cpp b/Machines/Oric/Microdisc.cpp index 25844d6cd..1e2810db7 100644 --- a/Machines/Oric/Microdisc.cpp +++ b/Machines/Oric/Microdisc.cpp @@ -38,47 +38,60 @@ void Microdisc::set_disk(std::shared_ptr disk, int drive) void Microdisc::set_control_register(uint8_t control) { - printf("control: %d%d%d%d%d%d%d%d\n", - (control >> 7)&1, - (control >> 6)&1, - (control >> 5)&1, - (control >> 4)&1, - (control >> 3)&1, - (control >> 2)&1, - (control >> 1)&1, - (control >> 0)&1); +// printf("control: %d%d%d%d%d%d%d%d\n", +// (control >> 7)&1, +// (control >> 6)&1, +// (control >> 5)&1, +// (control >> 4)&1, +// (control >> 3)&1, +// (control >> 2)&1, +// (control >> 1)&1, +// (control >> 0)&1); + uint8_t changes = last_control_ ^ control; + last_control_ = control; // b2: data separator clock rate select (1 = double) [TODO] // b65: drive select - selected_drive_ = (control >> 5)&3; - set_drive(drives_[selected_drive_]); + if((changes >> 5)&3) + { + selected_drive_ = (control >> 5)&3; + set_drive(drives_[selected_drive_]); + } // b4: side select - unsigned int head = (control & 0x10) ? 1 : 0; - for(int c = 0; c < 4; c++) + if(changes & 0x10) { - if(drives_[c]) drives_[c]->set_head(head); + unsigned int head = (control & 0x10) ? 1 : 0; + for(int c = 0; c < 4; c++) + { + if(drives_[c]) drives_[c]->set_head(head); + } } // b3: double density select (0 = double) - set_is_double_density(!(control & 0x08)); + if(changes & 0x08) + { + set_is_double_density(!(control & 0x08)); + } // b0: IRQ enable - bool had_irq = get_interrupt_request_line(); - irq_enable_ = !!(control & 0x01); - bool has_irq = get_interrupt_request_line(); - if(has_irq != had_irq && delegate_) + if(changes & 0x01) { - delegate_->wd1770_did_change_output(this); + bool had_irq = get_interrupt_request_line(); + irq_enable_ = !!(control & 0x01); + bool has_irq = get_interrupt_request_line(); + if(has_irq != had_irq && delegate_) + { + delegate_->wd1770_did_change_output(this); + } } // b7: EPROM select (0 = select) // b1: ROM disable (0 = disable) - int new_paging_flags = ((control & 0x02) ? 0 : BASICDisable) | ((control & 0x80) ? MicrodscDisable : 0); - if(new_paging_flags != paging_flags_) + if(changes & 0x82) { - paging_flags_ = new_paging_flags; + paging_flags_ = ((control & 0x02) ? 0 : BASICDisable) | ((control & 0x80) ? MicrodscDisable : 0); if(delegate_) delegate_->microdisc_did_change_paging_flags(this); } } diff --git a/Machines/Oric/Microdisc.hpp b/Machines/Oric/Microdisc.hpp index 521ab2e7e..0b70afbfc 100644 --- a/Machines/Oric/Microdisc.hpp +++ b/Machines/Oric/Microdisc.hpp @@ -47,6 +47,7 @@ class Microdisc: public WD::WD1770 { int paging_flags_; int head_load_request_counter_; Delegate *delegate_; + uint8_t last_control_; }; } diff --git a/NumberTheory/CRC.hpp b/NumberTheory/CRC.hpp index 007b63096..038c27332 100644 --- a/NumberTheory/CRC.hpp +++ b/NumberTheory/CRC.hpp @@ -29,6 +29,7 @@ class CRC16 { } } inline uint16_t get_value() const { return value_; } + inline void set_value(uint16_t value) { value_ = value; } private: uint16_t reset_value_, polynomial_; diff --git a/OSBindings/Mac/Clock SignalTests/CRCTests.mm b/OSBindings/Mac/Clock SignalTests/CRCTests.mm index f5dc603b8..a370149d6 100644 --- a/OSBindings/Mac/Clock SignalTests/CRCTests.mm +++ b/OSBindings/Mac/Clock SignalTests/CRCTests.mm @@ -22,7 +22,8 @@ - (uint16_t)crcOfData:(uint8_t *)data length:(size_t)length generator:(NumberTheory::CRC16 &)generator { generator.reset(); - for(size_t c = 0; c < length; c++) generator.add(data[c]); + for(size_t c = 0; c < length; c++) + generator.add(data[c]); return generator.get_value(); } diff --git a/Storage/Disk/Encodings/MFM.cpp b/Storage/Disk/Encodings/MFM.cpp index 69767ad20..3bbb786e9 100644 --- a/Storage/Disk/Encodings/MFM.cpp +++ b/Storage/Disk/Encodings/MFM.cpp @@ -18,6 +18,7 @@ class MFMEncoder: public Encoder { MFMEncoder(std::vector &target) : Encoder(target) {} void add_byte(uint8_t input) { + crc_generator_.add(input); uint16_t spread_value = (uint16_t)( ((input & 0x01) << 0) | @@ -29,33 +30,42 @@ class MFMEncoder: public Encoder { ((input & 0x40) << 6) | ((input & 0x80) << 7) ); - uint16_t or_bits = (uint16_t)((spread_value << 1) | (spread_value >> 1) | (output_ << 15)); - output_ = spread_value | ((~or_bits) & 0xaaaa); - output_short(output_); + uint16_t or_bits = (uint16_t)((spread_value << 1) | (spread_value >> 1) | (last_output_ << 15)); + uint16_t output = spread_value | ((~or_bits) & 0xaaaa); + output_short(output); } void add_index_address_mark() { - output_short(output_ = MFMIndexAddressMark); + for(int c = 0; c < 3; c++) output_short(MFMIndexSync); add_byte(MFMIndexAddressByte); } void add_ID_address_mark() { - output_short(output_ = MFMAddressMark); + output_sync(); add_byte(MFMIDAddressByte); } void add_data_address_mark() { - output_short(output_ = MFMAddressMark); + output_sync(); add_byte(MFMDataAddressByte); } void add_deleted_data_address_mark() { - output_short(output_ = MFMAddressMark); + output_sync(); add_byte(MFMDeletedDataAddressByte); } private: - uint16_t output_; + uint16_t last_output_; + void output_short(uint16_t value) { + last_output_ = value; + Encoder::output_short(value); + } + + void output_sync() { + for(int c = 0; c < 3; c++) output_short(MFMSync); + crc_generator_.set_value(MFMPostSyncCRCValue); + } }; class FMEncoder: public Encoder { @@ -64,6 +74,7 @@ class FMEncoder: public Encoder { FMEncoder(std::vector &target) : Encoder(target) {} void add_byte(uint8_t input) { + crc_generator_.add(input); output_short( (uint16_t)( ((input & 0x01) << 0) | @@ -109,7 +120,6 @@ template std::shared_ptr Storage::Disk::PCMSegment segment; segment.data.reserve(expected_track_bytes); T shifter(segment.data); - NumberTheory::CRC16 crc_generator(0x1021, 0xffff); // output the index mark shifter.add_index_address_mark(); @@ -130,16 +140,7 @@ template std::shared_ptr shifter.add_byte(sector.sector); uint8_t size = logarithmic_size_for_size(sector.data.size()); shifter.add_byte(size); - - // header CRC - crc_generator.reset(); - crc_generator.add(sector.track); - crc_generator.add(sector.side); - crc_generator.add(sector.sector); - crc_generator.add(size); - uint16_t crc_value = crc_generator.get_value(); - shifter.add_byte(crc_value >> 8); - shifter.add_byte(crc_value & 0xff); + shifter.add_crc(); // gap for(int c = 0; c < post_address_mark_bytes; c++) shifter.add_byte(0x4e); @@ -147,17 +148,11 @@ template std::shared_ptr // data shifter.add_data_address_mark(); - crc_generator.reset(); for(size_t c = 0; c < sector.data.size(); c++) { shifter.add_byte(sector.data[c]); - crc_generator.add(sector.data[c]); } - - // data CRC - crc_value = crc_generator.get_value(); - shifter.add_byte(crc_value >> 8); - shifter.add_byte(crc_value & 0xff); + shifter.add_crc(); // gap for(int c = 0; c < post_data_bytes; c++) shifter.add_byte(0x00); @@ -171,6 +166,7 @@ template std::shared_ptr } Encoder::Encoder(std::vector &target) : + crc_generator_(0x1021, 0xffff), target_(target) {} @@ -180,6 +176,11 @@ void Encoder::output_short(uint16_t value) target_.push_back(value & 0xff); } +void Encoder::add_crc() +{ + output_short(crc_generator_.get_value()); +} + std::shared_ptr Storage::Encodings::MFM::GetFMTrackWithSectors(const std::vector §ors) { return GetTrackWithSectors( @@ -298,7 +299,7 @@ std::shared_ptr Parser::get_next_sector() run_for_cycles(1); if(is_mfm_) { - if(shift_register_ == Storage::Encodings::MFM::MFMAddressMark) + while(shift_register_ == Storage::Encodings::MFM::MFMSync) { uint8_t mark = get_next_byte(); if(mark == Storage::Encodings::MFM::MFMIDAddressByte) break; @@ -326,7 +327,7 @@ std::shared_ptr Parser::get_next_sector() run_for_cycles(1); if(is_mfm_) { - if(shift_register_ == Storage::Encodings::MFM::MFMAddressMark) + while(shift_register_ == Storage::Encodings::MFM::MFMSync) { uint8_t mark = get_next_byte(); if(mark == Storage::Encodings::MFM::MFMDataAddressByte) break; diff --git a/Storage/Disk/Encodings/MFM.hpp b/Storage/Disk/Encodings/MFM.hpp index 5f26ec7c0..ff271afc0 100644 --- a/Storage/Disk/Encodings/MFM.hpp +++ b/Storage/Disk/Encodings/MFM.hpp @@ -24,12 +24,13 @@ const uint16_t FMIDAddressMark = 0xf57e; // data 0xfe, with clock 0xc7 => 1111 const uint16_t FMDataAddressMark = 0xf56f; // data 0xfb, with clock 0xc7 => 1111 1011 with clock 1100 0111 => 1111 0101 0110 1111 const uint16_t FMDeletedDataAddressMark = 0xf56a; // data 0xf8, with clock 0xc7 => 1111 1000 with clock 1100 0111 => 1111 0101 0110 1010 -const uint16_t MFMIndexAddressMark = 0x5224; -const uint16_t MFMAddressMark = 0x4489; +const uint16_t MFMIndexSync = 0x5224; +const uint16_t MFMSync = 0x4489; const uint8_t MFMIndexAddressByte = 0xfc; const uint8_t MFMIDAddressByte = 0xfe; const uint8_t MFMDataAddressByte = 0xfb; const uint8_t MFMDeletedDataAddressByte = 0xf8; +const uint16_t MFMPostSyncCRCValue = 0xcdb4; struct Sector { uint8_t track, side, sector; @@ -47,9 +48,11 @@ class Encoder { virtual void add_ID_address_mark() = 0; virtual void add_data_address_mark() = 0; virtual void add_deleted_data_address_mark() = 0; + void add_crc(); protected: - void output_short(uint16_t value); + virtual void output_short(uint16_t value); + NumberTheory::CRC16 crc_generator_; private: std::vector &target_; From 7a627b782df50837b173aefb6695469354508338 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 28 Dec 2016 18:48:50 -0500 Subject: [PATCH 23/52] Reintroduced writing of MFM sync marks when writing a sector. --- Components/1770/1770.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/Components/1770/1770.cpp b/Components/1770/1770.cpp index 7ea6a4b90..4ced073c8 100644 --- a/Components/1770/1770.cpp +++ b/Components/1770/1770.cpp @@ -651,6 +651,7 @@ void WD1770::posit_event(Event new_event_type) if(is_double_density_) { crc_generator_.set_value(Storage::Encodings::MFM::MFMPostSyncCRCValue); + for(int c = 0; c < 3; c++) write_raw_short(Storage::Encodings::MFM::MFMSync); write_byte((command_&0x01) ? Storage::Encodings::MFM::MFMDeletedDataAddressByte : Storage::Encodings::MFM::MFMDataAddressByte); } else From 90151e209492d4ab28c5be9c8f60866be69e5134 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 28 Dec 2016 18:49:32 -0500 Subject: [PATCH 24/52] Fixed to ensure a known initial control register value, which has taken effect. --- Machines/Oric/Microdisc.cpp | 20 +++++++++----------- Machines/Oric/Microdisc.hpp | 1 + 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/Machines/Oric/Microdisc.cpp b/Machines/Oric/Microdisc.cpp index 1e2810db7..79d16ebd4 100644 --- a/Machines/Oric/Microdisc.cpp +++ b/Machines/Oric/Microdisc.cpp @@ -23,8 +23,11 @@ Microdisc::Microdisc() : delegate_(nullptr), paging_flags_(BASICDisable), head_load_request_counter_(-1), - WD1770(P1793) -{} + WD1770(P1793), + last_control_(0) +{ + set_control_register(last_control_, 0xff); +} void Microdisc::set_disk(std::shared_ptr disk, int drive) { @@ -38,18 +41,13 @@ void Microdisc::set_disk(std::shared_ptr disk, int drive) void Microdisc::set_control_register(uint8_t control) { -// printf("control: %d%d%d%d%d%d%d%d\n", -// (control >> 7)&1, -// (control >> 6)&1, -// (control >> 5)&1, -// (control >> 4)&1, -// (control >> 3)&1, -// (control >> 2)&1, -// (control >> 1)&1, -// (control >> 0)&1); uint8_t changes = last_control_ ^ control; last_control_ = control; + set_control_register(control, changes); +} +void Microdisc::set_control_register(uint8_t control, uint8_t changes) +{ // b2: data separator clock rate select (1 = double) [TODO] // b65: drive select diff --git a/Machines/Oric/Microdisc.hpp b/Machines/Oric/Microdisc.hpp index 0b70afbfc..d320c7721 100644 --- a/Machines/Oric/Microdisc.hpp +++ b/Machines/Oric/Microdisc.hpp @@ -39,6 +39,7 @@ class Microdisc: public WD::WD1770 { inline int get_paging_flags() { return paging_flags_; } private: + void set_control_register(uint8_t control, uint8_t changes); void set_head_load_request(bool head_load); bool get_drive_is_ready(); std::shared_ptr drives_[4]; From 3a9ad3fb087c075be417f26ff2a0642b11be16c0 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 28 Dec 2016 18:50:28 -0500 Subject: [PATCH 25/52] Fixed Oric .DSK handling, per my latest understanding. Which creates a desire to write shorts directly to the disk surface, so exposed that in the encoder. --- Storage/Disk/Encodings/MFM.hpp | 2 +- Storage/Disk/Formats/OricMFMDSK.cpp | 53 ++++++++++++++++++----------- 2 files changed, 35 insertions(+), 20 deletions(-) diff --git a/Storage/Disk/Encodings/MFM.hpp b/Storage/Disk/Encodings/MFM.hpp index ff271afc0..f1545407d 100644 --- a/Storage/Disk/Encodings/MFM.hpp +++ b/Storage/Disk/Encodings/MFM.hpp @@ -48,10 +48,10 @@ class Encoder { virtual void add_ID_address_mark() = 0; virtual void add_data_address_mark() = 0; virtual void add_deleted_data_address_mark() = 0; + virtual void output_short(uint16_t value); void add_crc(); protected: - virtual void output_short(uint16_t value); NumberTheory::CRC16 crc_generator_; private: diff --git a/Storage/Disk/Formats/OricMFMDSK.cpp b/Storage/Disk/Formats/OricMFMDSK.cpp index 7ab373bd7..c0de30f9e 100644 --- a/Storage/Disk/Formats/OricMFMDSK.cpp +++ b/Storage/Disk/Formats/OricMFMDSK.cpp @@ -57,6 +57,7 @@ std::shared_ptr OricMFMDSK::get_uncached_track_at_position(unsigned int h size_t track_offset = 0; uint8_t last_header[6]; std::unique_ptr encoder = Encodings::MFM::GetMFMEncoder(segment.data); + bool did_sync = false; while(track_offset < 6250) { uint8_t next_byte = (uint8_t)fgetc(file_); @@ -65,32 +66,46 @@ std::shared_ptr OricMFMDSK::get_uncached_track_at_position(unsigned int h switch(next_byte) { default: - encoder->add_byte(next_byte); - break; - - case 0xfe: // an ID synchronisation { - encoder->add_ID_address_mark(); - - for(int byte = 0; byte < 6; byte++) + encoder->add_byte(next_byte); + if(did_sync) { - last_header[byte] = (uint8_t)fgetc(file_); - encoder->add_byte(last_header[byte]); - track_offset++; - if(track_offset == 6250) break; + switch(next_byte) + { + default: break; + + case 0xfe: + for(int byte = 0; byte < 6; byte++) + { + last_header[byte] = (uint8_t)fgetc(file_); + encoder->add_byte(last_header[byte]); + track_offset++; + if(track_offset == 6250) break; + } + break; + + case 0xfb: + for(int byte = 0; byte < (128 << last_header[3]) + 2; byte++) + { + encoder->add_byte((uint8_t)fgetc(file_)); + track_offset++; + if(track_offset == 6250) break; + } + break; + } } + + did_sync = false; } break; - case 0xfb: // a data synchronisation - encoder->add_data_address_mark(); + case 0xa1: // a synchronisation mark that implies a sector or header coming + encoder->output_short(Storage::Encodings::MFM::MFMSync); + did_sync = true; + break; - for(int byte = 0; byte < (128 << last_header[3]) + 2; byte++) - { - encoder->add_byte((uint8_t)fgetc(file_)); - track_offset++; - if(track_offset == 6250) break; - } + case 0xc2: // an 'ordinary' synchronisation mark + encoder->output_short(Storage::Encodings::MFM::MFMIndexSync); break; } } From 8cd1575891b4a221433fe814af2ac67ea8feceb5 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 28 Dec 2016 18:52:36 -0500 Subject: [PATCH 26/52] Similar fix to that over in Oric land: ensure a known, effective initial value for the Plus 3's control register. --- Machines/Electron/Plus3.cpp | 9 ++++++++- Machines/Electron/Plus3.hpp | 1 + 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/Machines/Electron/Plus3.cpp b/Machines/Electron/Plus3.cpp index 9100a860c..d026f6bf9 100644 --- a/Machines/Electron/Plus3.cpp +++ b/Machines/Electron/Plus3.cpp @@ -10,7 +10,10 @@ using namespace Electron; -Plus3::Plus3() : WD1770(P1770) {} +Plus3::Plus3() : WD1770(P1770), last_control_(0) +{ + set_control_register(last_control_, 0xff); +} void Plus3::set_disk(std::shared_ptr disk, int drive) { @@ -31,7 +34,11 @@ void Plus3::set_control_register(uint8_t control) uint8_t changes = control ^ last_control_; last_control_ = control; + set_control_register(control, changes); +} +void Plus3::set_control_register(uint8_t control, uint8_t changes) +{ if(changes&3) { switch(control&3) diff --git a/Machines/Electron/Plus3.hpp b/Machines/Electron/Plus3.hpp index 8e6b7c0e4..37cf5b324 100644 --- a/Machines/Electron/Plus3.hpp +++ b/Machines/Electron/Plus3.hpp @@ -21,6 +21,7 @@ class Plus3 : public WD::WD1770 { void set_control_register(uint8_t control); private: + void set_control_register(uint8_t control, uint8_t changes); std::shared_ptr drives_[2]; int selected_drive_; uint8_t last_control_; From 720b1e580233915d2e9a49665b04400ee50584ff Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 28 Dec 2016 18:56:53 -0500 Subject: [PATCH 27/52] Attempted to ensure proper CRC generation for FM-format input. --- Storage/Disk/Encodings/MFM.cpp | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/Storage/Disk/Encodings/MFM.cpp b/Storage/Disk/Encodings/MFM.cpp index 3bbb786e9..60799bc4c 100644 --- a/Storage/Disk/Encodings/MFM.cpp +++ b/Storage/Disk/Encodings/MFM.cpp @@ -89,10 +89,33 @@ class FMEncoder: public Encoder { )); } - void add_index_address_mark() { output_short(FMIndexAddressMark); } - void add_ID_address_mark() { output_short(FMIDAddressMark); } - void add_data_address_mark() { output_short(FMDataAddressMark); } - void add_deleted_data_address_mark() { output_short(FMDeletedDataAddressMark); } + void add_index_address_mark() + { + crc_generator_.reset(); + crc_generator_.add(MFMIndexAddressByte); + output_short(FMIndexAddressMark); + } + + void add_ID_address_mark() + { + crc_generator_.reset(); + crc_generator_.add(MFMIDAddressByte); + output_short(FMIDAddressMark); + } + + void add_data_address_mark() + { + crc_generator_.reset(); + crc_generator_.add(MFMDataAddressByte); + output_short(FMDataAddressMark); + } + + void add_deleted_data_address_mark() + { + crc_generator_.reset(); + crc_generator_.add(MFMDeletedDataAddressByte); + output_short(FMDeletedDataAddressMark); + } }; static uint8_t logarithmic_size_for_size(size_t size) From 1277a67f9a49e680d410cb10a85cc3b06a865dad Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 28 Dec 2016 19:26:21 -0500 Subject: [PATCH 28/52] Introduced data_mode_ to replace is_reading_data_, representing that there are now three possible modes. When writing, any input from the read head won't affect the CRC generator. --- Components/1770/1770.cpp | 17 ++++++++++------- Components/1770/1770.hpp | 6 +++++- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/Components/1770/1770.cpp b/Components/1770/1770.cpp index 4ced073c8..afca995dd 100644 --- a/Components/1770/1770.cpp +++ b/Components/1770/1770.cpp @@ -35,7 +35,7 @@ WD1770::WD1770(Personality p) : delay_time_(0), index_hole_count_target_(-1), is_awaiting_marker_value_(false), - is_reading_data_(false), + data_mode_(DataMode::Scanning), delegate_(nullptr), personality_(p), head_is_loaded_(false) @@ -163,11 +163,13 @@ void WD1770::run_for_cycles(unsigned int number_of_cycles) void WD1770::process_input_bit(int value, unsigned int cycles_since_index_hole) { + if(data_mode_ == DataMode::Writing) return; + shift_register_ = (shift_register_ << 1) | value; bits_since_token_++; Token::Type token_type = Token::Byte; - if(!is_reading_data_) + if(data_mode_ == DataMode::Scanning) { if(!is_double_density_) { @@ -312,7 +314,7 @@ void WD1770::process_write_completed() #define READ_ID() \ if(new_event_type == Event::Token) \ { \ - if(!distance_into_section_ && latest_token_.type == Token::ID) {is_reading_data_ = true; distance_into_section_++; } \ + if(!distance_into_section_ && latest_token_.type == Token::ID) {data_mode_ = DataMode::Reading; distance_into_section_++; } \ else if(distance_into_section_ && distance_into_section_ < 7 && latest_token_.type == Token::Byte) \ { \ header_[distance_into_section_ - 1] = latest_token_.byte_value; \ @@ -343,7 +345,7 @@ void WD1770::posit_event(Event new_event_type) // Wait for a new command, branch to the appropriate handler. wait_for_command: printf("Idle...\n"); - is_reading_data_ = false; + data_mode_ = DataMode::Scanning; index_hole_count_ = 0; update_status([] (Status &status) { @@ -463,7 +465,7 @@ void WD1770::posit_event(Event new_event_type) } if(distance_into_section_ == 7) { - is_reading_data_ = false; + data_mode_ = DataMode::Scanning; // TODO: CRC check if(header_[0] == track_) { @@ -539,7 +541,7 @@ void WD1770::posit_event(Event new_event_type) if(distance_into_section_ == 7) { printf("Considering %d/%d\n", header_[0], header_[2]); - is_reading_data_ = false; + data_mode_ = DataMode::Scanning; if(header_[0] == track_ && header_[2] == sector_ && (has_motor_on_line() || !(command_&0x02) || ((command_&0x08) >> 3) == header_[1])) { @@ -575,7 +577,7 @@ void WD1770::posit_event(Event new_event_type) status.record_type = (latest_token_.type == Token::DeletedData); }); distance_into_section_ = 0; - is_reading_data_ = true; + data_mode_ = DataMode::Reading; goto type2_read_byte; } goto type2_read_data; @@ -641,6 +643,7 @@ void WD1770::posit_event(Event new_event_type) WAIT_FOR_BYTES(11); } + data_mode_ = DataMode::Writing; begin_writing(); for(int c = 0; c < (is_double_density_ ? 12 : 6); c++) { diff --git a/Components/1770/1770.hpp b/Components/1770/1770.hpp index 92211dbde..e06bcac8b 100644 --- a/Components/1770/1770.hpp +++ b/Components/1770/1770.hpp @@ -95,7 +95,11 @@ class WD1770: public Storage::Disk::Controller { void update_status(std::function updater); // Tokeniser - bool is_reading_data_; + enum DataMode { + Scanning, + Reading, + Writing + } data_mode_; bool is_double_density_; int shift_register_; struct Token { From 46a93d2e12d054f0ccf5fc3517d660271fa1605a Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 28 Dec 2016 19:48:46 -0500 Subject: [PATCH 29/52] Fixed errors to ensure that FM disks, at least, follow the same CRC generation rules when being built from sectors and when being parsed. --- Storage/Disk/Encodings/MFM.cpp | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/Storage/Disk/Encodings/MFM.cpp b/Storage/Disk/Encodings/MFM.cpp index 60799bc4c..0fa77b9ff 100644 --- a/Storage/Disk/Encodings/MFM.cpp +++ b/Storage/Disk/Encodings/MFM.cpp @@ -201,7 +201,9 @@ void Encoder::output_short(uint16_t value) void Encoder::add_crc() { - output_short(crc_generator_.get_value()); + uint16_t crc_value = crc_generator_.get_value(); + add_byte(crc_value >> 8); + add_byte(crc_value & 0xff); } std::shared_ptr Storage::Encodings::MFM::GetFMTrackWithSectors(const std::vector §ors) @@ -325,17 +327,25 @@ std::shared_ptr Parser::get_next_sector() while(shift_register_ == Storage::Encodings::MFM::MFMSync) { uint8_t mark = get_next_byte(); - if(mark == Storage::Encodings::MFM::MFMIDAddressByte) break; + if(mark == Storage::Encodings::MFM::MFMIDAddressByte) + { + crc_generator_.set_value(MFMPostSyncCRCValue); + break; + } } } else { - if(shift_register_ == Storage::Encodings::MFM::FMIDAddressMark) break; + if(shift_register_ == Storage::Encodings::MFM::FMIDAddressMark) + { + crc_generator_.reset(); + break; + } } if(index_count_ >= 2) return nullptr; } - crc_generator_.reset(); + crc_generator_.add(MFMIDAddressByte); sector->track = get_next_byte(); sector->side = get_next_byte(); sector->sector = get_next_byte(); @@ -353,21 +363,29 @@ std::shared_ptr Parser::get_next_sector() while(shift_register_ == Storage::Encodings::MFM::MFMSync) { uint8_t mark = get_next_byte(); - if(mark == Storage::Encodings::MFM::MFMDataAddressByte) break; + if(mark == Storage::Encodings::MFM::MFMDataAddressByte) + { + crc_generator_.set_value(MFMPostSyncCRCValue); + break; + } if(mark == Storage::Encodings::MFM::MFMIDAddressByte) return nullptr; } } else { - if(shift_register_ == Storage::Encodings::MFM::FMDataAddressMark) break; + if(shift_register_ == Storage::Encodings::MFM::FMDataAddressMark) + { + crc_generator_.reset(); + break; + } if(shift_register_ == Storage::Encodings::MFM::FMIDAddressMark) return nullptr; } if(index_count_ >= 2) return nullptr; } + crc_generator_.add(MFMDataAddressByte); size_t data_size = (size_t)(128 << size); sector->data.reserve(data_size); - crc_generator_.reset(); for(size_t c = 0; c < data_size; c++) { sector->data.push_back(get_next_byte()); From 4adcb46665d198538e245ac4b9ca71611639c20c Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 28 Dec 2016 19:51:27 -0500 Subject: [PATCH 30/52] Fixed FM-mode CRC generation. --- Components/1770/1770.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Components/1770/1770.cpp b/Components/1770/1770.cpp index afca995dd..fa79d846e 100644 --- a/Components/1770/1770.cpp +++ b/Components/1770/1770.cpp @@ -177,15 +177,23 @@ void WD1770::process_input_bit(int value, unsigned int cycles_since_index_hole) { case Storage::Encodings::MFM::FMIndexAddressMark: token_type = Token::Index; + crc_generator_.reset(); + crc_generator_.add(Storage::Encodings::MFM::MFMIndexAddressByte); break; case Storage::Encodings::MFM::FMIDAddressMark: token_type = Token::ID; + crc_generator_.reset(); + crc_generator_.add(Storage::Encodings::MFM::MFMIDAddressByte); break; case Storage::Encodings::MFM::FMDataAddressMark: token_type = Token::Data; + crc_generator_.reset(); + crc_generator_.add(Storage::Encodings::MFM::MFMDataAddressByte); break; case Storage::Encodings::MFM::FMDeletedDataAddressMark: token_type = Token::DeletedData; + crc_generator_.reset(); + crc_generator_.add(Storage::Encodings::MFM::MFMDeletedDataAddressByte); break; default: break; From bfe6c0a0c1d70fff61f3b83e737ff03d84de40f8 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 28 Dec 2016 20:09:14 -0500 Subject: [PATCH 31/52] Ensured that `FileHolder` gets a writeable file reference if one is possible, and records whether the file in hand is read-only. So now the SSD class can answer honestly. --- Storage/Disk/Formats/SSD.cpp | 11 +++++++---- Storage/FileHolder.cpp | 8 +++++++- Storage/FileHolder.hpp | 1 + 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/Storage/Disk/Formats/SSD.cpp b/Storage/Disk/Formats/SSD.cpp index 4f6863214..7493c2419 100644 --- a/Storage/Disk/Formats/SSD.cpp +++ b/Storage/Disk/Formats/SSD.cpp @@ -65,7 +65,7 @@ unsigned int SSD::get_head_count() bool SSD::get_is_read_only() { - return false; + return is_read_only_; } std::shared_ptr SSD::get_uncached_track_at_position(unsigned int head, unsigned int position) @@ -85,9 +85,12 @@ std::shared_ptr SSD::get_uncached_track_at_position(unsigned int head, un new_sector.sector = (uint8_t)sector; new_sector.data.resize(256); - fread(&new_sector.data[0], 1, 256, file_); -// if(feof(file_)) -// new_sector.data[0]; + fread(new_sector.data.data(), 1, 256, file_); + + // zero out if this wasn't present in the disk image; it's still appropriate to put a sector + // on disk because one will have been placed during formatting, but there's no reason to leak + // information from outside the emulated machine's world + if(feof(file_)) memset(new_sector.data.data(), 0, 256); sectors.push_back(std::move(new_sector)); } diff --git a/Storage/FileHolder.cpp b/Storage/FileHolder.cpp index b5bd16bb3..6271a79cd 100644 --- a/Storage/FileHolder.cpp +++ b/Storage/FileHolder.cpp @@ -20,7 +20,13 @@ FileHolder::~FileHolder() FileHolder::FileHolder(const char *file_name) : file_(nullptr) { stat(file_name, &file_stats_); - file_ = fopen(file_name, "rb"); + is_read_only_ = false; + file_ = fopen(file_name, "rb+"); + if(!file_) + { + is_read_only_ = true; + file_ = fopen(file_name, "rb"); + } if(!file_) throw ErrorCantOpen; } diff --git a/Storage/FileHolder.hpp b/Storage/FileHolder.hpp index ef853a844..bf1029102 100644 --- a/Storage/FileHolder.hpp +++ b/Storage/FileHolder.hpp @@ -67,6 +67,7 @@ class FileHolder { FILE *file_; struct stat file_stats_; + bool is_read_only_; }; } From ce814c9e9982029dfc21f0aa62abb3347b198850 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 28 Dec 2016 21:23:22 -0500 Subject: [PATCH 32/52] These can be const. --- NumberTheory/CRC.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NumberTheory/CRC.hpp b/NumberTheory/CRC.hpp index 038c27332..65bb0477e 100644 --- a/NumberTheory/CRC.hpp +++ b/NumberTheory/CRC.hpp @@ -32,7 +32,7 @@ class CRC16 { inline void set_value(uint16_t value) { value_ = value; } private: - uint16_t reset_value_, polynomial_; + const uint16_t reset_value_, polynomial_; uint16_t value_; }; From e4000bd0604f963992b30bcb0815c55a5d5d5a8c Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 28 Dec 2016 21:24:19 -0500 Subject: [PATCH 33/52] Added some even more verbose logging; slightly simplified write loop logic, and decided it's definitely `write_byte` that's responsible for CRC generator feeding. --- Components/1770/1770.cpp | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/Components/1770/1770.cpp b/Components/1770/1770.cpp index fa79d846e..23caba73d 100644 --- a/Components/1770/1770.cpp +++ b/Components/1770/1770.cpp @@ -556,6 +556,7 @@ void WD1770::posit_event(Event new_event_type) printf("Found %d/%d\n", header_[0], header_[2]); if(crc_generator_.get_value()) { + printf("CRC error; back to searching\n"); update_status([] (Status &status) { status.crc_error = true; }); @@ -615,6 +616,7 @@ void WD1770::posit_event(Event new_event_type) { if(crc_generator_.get_value()) { + printf("CRC error; terminating\n"); update_status([this] (Status &status) { status.crc_error = true; }); @@ -684,17 +686,17 @@ void WD1770::posit_event(Event new_event_type) natural expectations and the way that emulated machines responded, I believe that to be a documentation error. */ - crc_generator_.add(data_); write_byte(data_); distance_into_section_++; - if(distance_into_section_ < 128 << header_[3]) + if(distance_into_section_ == 128 << header_[3]) { - update_status([] (Status &status) { - status.data_request = true; - }); + goto type2_write_crc; } - WAIT_FOR_EVENT(Event::DataWritten); + update_status([] (Status &status) { + status.data_request = true; + }); + WAIT_FOR_EVENT(Event::DataWritten); if(status_.data_request) { update_status([] (Status &status) { @@ -703,17 +705,13 @@ void WD1770::posit_event(Event new_event_type) goto wait_for_command; } - if(distance_into_section_ == 128 << header_[3]) - { - goto type2_write_crc; - } goto type2_write_loop; type2_write_crc: { uint16_t crc = crc_generator_.get_value(); write_byte(crc >> 8); - write_byte((crc & 0xff)); + write_byte(crc & 0xff); } write_byte(0xff); WAIT_FOR_EVENT(Event::DataWritten); From 9cb902cc4fe4558a116843091296632a18db5ae6 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 28 Dec 2016 22:34:22 -0500 Subject: [PATCH 34/52] Experimentally marked ADF as writable too, immediately discovering a mistake in the analysing MFM decoder. --- Storage/Disk/Encodings/MFM.cpp | 12 ++++++++---- Storage/Disk/Formats/AcornADF.cpp | 5 +++++ Storage/Disk/Formats/AcornADF.hpp | 1 + 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/Storage/Disk/Encodings/MFM.cpp b/Storage/Disk/Encodings/MFM.cpp index 0fa77b9ff..102815d07 100644 --- a/Storage/Disk/Encodings/MFM.cpp +++ b/Storage/Disk/Encodings/MFM.cpp @@ -319,7 +319,8 @@ std::shared_ptr Parser::get_next_sector() while(index_count_ < 2) { // look for an ID address mark - while(1) + bool id_found = false; + while(!id_found) { run_for_cycles(1); if(is_mfm_) @@ -330,6 +331,7 @@ std::shared_ptr Parser::get_next_sector() if(mark == Storage::Encodings::MFM::MFMIDAddressByte) { crc_generator_.set_value(MFMPostSyncCRCValue); + id_found = true; break; } } @@ -339,7 +341,7 @@ std::shared_ptr Parser::get_next_sector() if(shift_register_ == Storage::Encodings::MFM::FMIDAddressMark) { crc_generator_.reset(); - break; + id_found = true; } } if(index_count_ >= 2) return nullptr; @@ -355,7 +357,8 @@ std::shared_ptr Parser::get_next_sector() if((header_crc & 0xff) != get_next_byte()) continue; // look for data mark - while(1) + bool data_found = false; + while(!data_found) { run_for_cycles(1); if(is_mfm_) @@ -366,6 +369,7 @@ std::shared_ptr Parser::get_next_sector() if(mark == Storage::Encodings::MFM::MFMDataAddressByte) { crc_generator_.set_value(MFMPostSyncCRCValue); + data_found = true; break; } if(mark == Storage::Encodings::MFM::MFMIDAddressByte) return nullptr; @@ -376,7 +380,7 @@ std::shared_ptr Parser::get_next_sector() if(shift_register_ == Storage::Encodings::MFM::FMDataAddressMark) { crc_generator_.reset(); - break; + data_found = true; } if(shift_register_ == Storage::Encodings::MFM::FMIDAddressMark) return nullptr; } diff --git a/Storage/Disk/Formats/AcornADF.cpp b/Storage/Disk/Formats/AcornADF.cpp index 5eb7d7dce..9efd45811 100644 --- a/Storage/Disk/Formats/AcornADF.cpp +++ b/Storage/Disk/Formats/AcornADF.cpp @@ -47,6 +47,11 @@ unsigned int AcornADF::get_head_count() return 1; } +bool AcornADF::get_is_read_only() +{ + return is_read_only_; +} + std::shared_ptr AcornADF::get_uncached_track_at_position(unsigned int head, unsigned int position) { std::shared_ptr track; diff --git a/Storage/Disk/Formats/AcornADF.hpp b/Storage/Disk/Formats/AcornADF.hpp index 11109d5ea..7be2d1b87 100644 --- a/Storage/Disk/Formats/AcornADF.hpp +++ b/Storage/Disk/Formats/AcornADF.hpp @@ -35,6 +35,7 @@ class AcornADF: public Disk, public Storage::FileHolder { // implemented to satisfy @c Disk unsigned int get_head_position_count(); unsigned int get_head_count(); + bool get_is_read_only(); private: std::shared_ptr get_uncached_track_at_position(unsigned int head, unsigned int position); }; From af1b396c9e33352b453ff38403f22edc3eb570a1 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 28 Dec 2016 22:57:11 -0500 Subject: [PATCH 35/52] Found an ugly issue with `Storage::Time` as implemented (i) to be unsigned; and (ii) automatically to simplify. Will need to fix. Here's a quick workaround for this one segment of code. --- Storage/Disk/PCMPatchedTrack.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Storage/Disk/PCMPatchedTrack.cpp b/Storage/Disk/PCMPatchedTrack.cpp index a70ec9da9..4356912c5 100644 --- a/Storage/Disk/PCMPatchedTrack.cpp +++ b/Storage/Disk/PCMPatchedTrack.cpp @@ -155,13 +155,17 @@ Track::Event PCMPatchedTrack::get_next_event() // see what time that gets us to. If it's still within the current period, return the found event Time event_time = current_time_ + event.length - period_error - insertion_error_; - insertion_error_.set_zero(); if(event_time < active_period_->end_time) { current_time_ = event_time; - event.length += extra_time - period_error; + // TODO: this is spelt out in three steps because times don't necessarily do the sensible + // thing when 'negative' if intermediate result get simplified in the meantime. So fix Time. + event.length += extra_time; + event.length -= period_error; + event.length -= insertion_error_; return event; } + insertion_error_.set_zero(); // otherwise move time back to the end of the outgoing period, accumulating the error into // extra_time, and advance the extra period From b9fad184d789f25f730ff612dcabd138472e428e Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 28 Dec 2016 23:00:47 -0500 Subject: [PATCH 36/52] Added just enough for a complete manual test of writing to a .ADF with the 1770 then getting the correct result parsing it back on the host side in order potentially to update a file. ... which means that now it's time to worry about when and how mounted files should actually update themselves. Which will make for some fun with threading, I dare say. --- Storage/Disk/Formats/AcornADF.cpp | 23 +++++++++++++++++++++++ Storage/Disk/Formats/AcornADF.hpp | 1 + 2 files changed, 24 insertions(+) diff --git a/Storage/Disk/Formats/AcornADF.cpp b/Storage/Disk/Formats/AcornADF.cpp index 9efd45811..b1e8d0794 100644 --- a/Storage/Disk/Formats/AcornADF.cpp +++ b/Storage/Disk/Formats/AcornADF.cpp @@ -37,6 +37,29 @@ AcornADF::AcornADF(const char *file_name) : if(bytes[0] != 'H' || bytes[1] != 'u' || bytes[2] != 'g' || bytes[3] != 'o') throw ErrorNotAcornADF; } +AcornADF::~AcornADF() +{ + if(get_is_modified()) + { + for(unsigned int head = 0; head < get_head_count(); head++) + { + for(unsigned int track = 0; track < get_head_position_count(); track++) + { + std::shared_ptr modified_track = get_modified_track_at_position(head, track); + if(modified_track) + { + Storage::Encodings::MFM::Parser parser(true, modified_track); + for(unsigned int c = 0; c < sectors_per_track; c++) + { + std::shared_ptr sector = parser.get_sector((uint8_t)track, (uint8_t)c); + printf("Sector %d: %p\n", c, sector.get()); + } + } + } + } + } +} + unsigned int AcornADF::get_head_position_count() { return 80; diff --git a/Storage/Disk/Formats/AcornADF.hpp b/Storage/Disk/Formats/AcornADF.hpp index 7be2d1b87..ea996303f 100644 --- a/Storage/Disk/Formats/AcornADF.hpp +++ b/Storage/Disk/Formats/AcornADF.hpp @@ -27,6 +27,7 @@ class AcornADF: public Disk, public Storage::FileHolder { @throws ErrorNotAcornADF if the file doesn't appear to contain an Acorn .ADF format image. */ AcornADF(const char *file_name); + ~AcornADF(); enum { ErrorNotAcornADF, From a8bc9d830ea75fc1deffb343e5234529849cd934 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 28 Dec 2016 23:03:05 -0500 Subject: [PATCH 37/52] Removed leftover very temporary debugging aid. --- Components/1770/1770.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/Components/1770/1770.cpp b/Components/1770/1770.cpp index 23caba73d..668fff112 100644 --- a/Components/1770/1770.cpp +++ b/Components/1770/1770.cpp @@ -11,8 +11,6 @@ using namespace WD; -unsigned int counter = 0; - WD1770::Status::Status() : type(Status::One), write_protect(false), @@ -144,7 +142,6 @@ uint8_t WD1770::get_register(int address) void WD1770::run_for_cycles(unsigned int number_of_cycles) { - counter += number_of_cycles; Storage::Disk::Controller::run_for_cycles((int)number_of_cycles); if(delay_time_) From 54900ca3fbc34225d95b54b111b6c9836c9d2e70 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Thu, 29 Dec 2016 11:00:47 -0500 Subject: [PATCH 38/52] Addition and subtraction can end immediately without performing any extra work if the operand is 0 --- Storage/Storage.hpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Storage/Storage.hpp b/Storage/Storage.hpp index 3feff4d5f..bc8d5a059 100644 --- a/Storage/Storage.hpp +++ b/Storage/Storage.hpp @@ -89,6 +89,8 @@ struct Time { inline Time operator + (const Time &other) const { + if(!other.length) return *this; + uint64_t result_length; uint64_t result_clock_rate; if(clock_rate == other.clock_rate) @@ -106,6 +108,8 @@ struct Time { inline Time &operator += (const Time &other) { + if(!other.length) return *this; + uint64_t result_length; uint64_t result_clock_rate; if(clock_rate == other.clock_rate) @@ -124,6 +128,8 @@ struct Time { inline Time operator - (const Time &other) const { + if(!other.length) return *this; + uint64_t result_length; uint64_t result_clock_rate; if(clock_rate == other.clock_rate) @@ -141,6 +147,8 @@ struct Time { inline Time operator -= (const Time &other) { + if(!other.length) return *this; + uint64_t result_length; uint64_t result_clock_rate; if(clock_rate == other.clock_rate) From bbd94749f4a73543a104d4487ac190b914bf77ce Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Thu, 29 Dec 2016 11:02:21 -0500 Subject: [PATCH 39/52] ... and I guess an instant maximal simplification is also easy if length ends up being 0 --- Storage/Storage.hpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Storage/Storage.hpp b/Storage/Storage.hpp index bc8d5a059..ea05faebc 100644 --- a/Storage/Storage.hpp +++ b/Storage/Storage.hpp @@ -246,6 +246,12 @@ struct Time { inline void install_result(uint64_t long_length, uint64_t long_clock_rate) { // TODO: switch to appropriate values if the result is too large or small to fit, even with trimmed accuracy. + if(!long_length) + { + length = 0; + clock_rate = 1; + return; + } while(!(long_length&1) && !(long_clock_rate&1)) { From 6fc692cd347b52f89a06489eaa1f04d95eed37d1 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Thu, 29 Dec 2016 22:15:58 -0500 Subject: [PATCH 40/52] Attempted to switch to an asynchronous means for continuous file updates. Testing with SSD, as usual. --- Storage/Disk/Disk.cpp | 20 ++++++------ Storage/Disk/Disk.hpp | 18 +++++------ Storage/Disk/Formats/AcornADF.cpp | 39 +++++++++++------------ Storage/Disk/Formats/SSD.cpp | 51 ++++++++++++++++++------------- Storage/Disk/Formats/SSD.hpp | 2 ++ 5 files changed, 69 insertions(+), 61 deletions(-) diff --git a/Storage/Disk/Disk.cpp b/Storage/Disk/Disk.cpp index 96953f5bd..6938014e7 100644 --- a/Storage/Disk/Disk.cpp +++ b/Storage/Disk/Disk.cpp @@ -21,7 +21,11 @@ void Disk::set_track_at_position(unsigned int head, unsigned int position, const int address = get_id_for_track_at_position(head, position); cached_tracks_[address] = track; - modified_tracks_.insert(address); + + if(!update_queue_) update_queue_.reset(new Concurrency::AsyncTaskQueue); + update_queue_->enqueue([this, head, position, track] { + store_updated_track_at_position(head, position, track, file_access_mutex_); + }); } std::shared_ptr Disk::get_track_at_position(unsigned int head, unsigned int position) @@ -30,21 +34,15 @@ std::shared_ptr Disk::get_track_at_position(unsigned int head, unsigned i std::map>::iterator cached_track = cached_tracks_.find(address); if(cached_track != cached_tracks_.end()) return cached_track->second; + std::lock_guard lock_guard(file_access_mutex_); std::shared_ptr track = get_uncached_track_at_position(head, position); cached_tracks_[address] = track; return track; } -std::shared_ptr Disk::get_modified_track_at_position(unsigned int head, unsigned int position) -{ - int address = get_id_for_track_at_position(head, position); - if(modified_tracks_.find(address) == modified_tracks_.end()) return nullptr; - std::map>::iterator cached_track = cached_tracks_.find(address); - if(cached_track == cached_tracks_.end()) return nullptr; - return cached_track->second; -} +void Disk::store_updated_track_at_position(unsigned int head, unsigned int position, const std::shared_ptr &track, std::mutex &file_access_mutex) {} -bool Disk::get_is_modified() +void Disk::flush_updates() { - return !modified_tracks_.empty(); + if(update_queue_) update_queue_->flush(); } diff --git a/Storage/Disk/Disk.hpp b/Storage/Disk/Disk.hpp index 7b435d838..b23081a04 100644 --- a/Storage/Disk/Disk.hpp +++ b/Storage/Disk/Disk.hpp @@ -9,10 +9,13 @@ #ifndef Disk_hpp #define Disk_hpp -#include #include +#include +#include #include + #include "../Storage.hpp" +#include "../../Concurrency/AsyncTaskQueue.hpp" namespace Storage { namespace Disk { @@ -107,20 +110,15 @@ class Disk { */ virtual std::shared_ptr get_uncached_track_at_position(unsigned int head, unsigned int position) = 0; - /*! - @returns @c true if any calls to set_track_at_position occurred; @c false otherwise. - */ - bool get_is_modified(); - /*! - @returns the @c Track at @c position underneath @c head if a modification was written there. - */ - std::shared_ptr get_modified_track_at_position(unsigned int head, unsigned int position); + virtual void store_updated_track_at_position(unsigned int head, unsigned int position, const std::shared_ptr &track, std::mutex &file_access_mutex); + void flush_updates(); private: std::map> cached_tracks_; - std::set modified_tracks_; int get_id_for_track_at_position(unsigned int head, unsigned int position); + std::mutex file_access_mutex_; + std::unique_ptr update_queue_; }; } diff --git a/Storage/Disk/Formats/AcornADF.cpp b/Storage/Disk/Formats/AcornADF.cpp index b1e8d0794..9615139f9 100644 --- a/Storage/Disk/Formats/AcornADF.cpp +++ b/Storage/Disk/Formats/AcornADF.cpp @@ -39,25 +39,26 @@ AcornADF::AcornADF(const char *file_name) : AcornADF::~AcornADF() { - if(get_is_modified()) - { - for(unsigned int head = 0; head < get_head_count(); head++) - { - for(unsigned int track = 0; track < get_head_position_count(); track++) - { - std::shared_ptr modified_track = get_modified_track_at_position(head, track); - if(modified_track) - { - Storage::Encodings::MFM::Parser parser(true, modified_track); - for(unsigned int c = 0; c < sectors_per_track; c++) - { - std::shared_ptr sector = parser.get_sector((uint8_t)track, (uint8_t)c); - printf("Sector %d: %p\n", c, sector.get()); - } - } - } - } - } + flush_updates(); +// if(get_is_modified()) +// { +// for(unsigned int head = 0; head < get_head_count(); head++) +// { +// for(unsigned int track = 0; track < get_head_position_count(); track++) +// { +// std::shared_ptr modified_track = get_modified_track_at_position(head, track); +// if(modified_track) +// { +// Storage::Encodings::MFM::Parser parser(true, modified_track); +// for(unsigned int c = 0; c < sectors_per_track; c++) +// { +// std::shared_ptr sector = parser.get_sector((uint8_t)track, (uint8_t)c); +// printf("Sector %d: %p\n", c, sector.get()); +// } +// } +// } +// } +// } } unsigned int AcornADF::get_head_position_count() diff --git a/Storage/Disk/Formats/SSD.cpp b/Storage/Disk/Formats/SSD.cpp index 7493c2419..b7977df58 100644 --- a/Storage/Disk/Formats/SSD.cpp +++ b/Storage/Disk/Formats/SSD.cpp @@ -32,27 +32,10 @@ SSD::SSD(const char *file_name) : SSD::~SSD() { - if(get_is_modified()) - { - for(unsigned int head = 0; head < head_count_; head++) - { - for(unsigned int track = 0; track < track_count_; track++) - { - std::shared_ptr modified_track = get_modified_track_at_position(head, track); - if(modified_track) - { - Storage::Encodings::MFM::Parser parser(false, modified_track); - for(unsigned int c = 0; c < 10; c++) - { - std::shared_ptr sector = parser.get_sector((uint8_t)track, (uint8_t)c); - printf("Sector %d: %p\n", c, sector.get()); - } - } - } - } - } + flush_updates(); } + unsigned int SSD::get_head_position_count() { return track_count_; @@ -68,13 +51,17 @@ bool SSD::get_is_read_only() return is_read_only_; } +long SSD::get_file_offset_for_position(unsigned int head, unsigned int position) +{ + return (position * head_count_ + head) * 256 * 10; +} + std::shared_ptr SSD::get_uncached_track_at_position(unsigned int head, unsigned int position) { std::shared_ptr track; if(head >= head_count_) return track; - long file_offset = (position * head_count_ + head) * 256 * 10; - fseek(file_, file_offset, SEEK_SET); + fseek(file_, get_file_offset_for_position(head, position), SEEK_SET); std::vector sectors; for(int sector = 0; sector < 10; sector++) @@ -99,3 +86,25 @@ std::shared_ptr SSD::get_uncached_track_at_position(unsigned int head, un return track; } + +void SSD::store_updated_track_at_position(unsigned int head, unsigned int position, const std::shared_ptr &track, std::mutex &file_access_mutex) +{ + std::vector parsed_track; + Storage::Encodings::MFM::Parser parser(false, track); + for(unsigned int c = 0; c < 10; c++) + { + std::shared_ptr sector = parser.get_sector((uint8_t)position, (uint8_t)c); + if(sector) + { + parsed_track.insert(parsed_track.end(), sector->data.begin(), sector->data.end()); + } + else + { + parsed_track.resize(parsed_track.size() + 256); + } + } + + std::lock_guard lock_guard(file_access_mutex); + fseek(file_, get_file_offset_for_position(head, position), SEEK_SET); + fwrite(parsed_track.data(), 1, parsed_track.size(), file_); +} diff --git a/Storage/Disk/Formats/SSD.hpp b/Storage/Disk/Formats/SSD.hpp index b7c21ccb3..0e806a147 100644 --- a/Storage/Disk/Formats/SSD.hpp +++ b/Storage/Disk/Formats/SSD.hpp @@ -39,7 +39,9 @@ class SSD: public Disk, public Storage::FileHolder { bool get_is_read_only(); private: + void store_updated_track_at_position(unsigned int head, unsigned int position, const std::shared_ptr &track, std::mutex &file_access_mutex); std::shared_ptr get_uncached_track_at_position(unsigned int head, unsigned int position); + long get_file_offset_for_position(unsigned int head, unsigned int position); unsigned int head_count_; unsigned int track_count_; }; From 82bb78fb2d852da7ca945e0fbda300713c6cea3e Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 30 Dec 2016 14:21:14 -0500 Subject: [PATCH 41/52] Ensured that get_sector copes even if any invalid sectors are encountered. --- Storage/Disk/Encodings/MFM.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/Storage/Disk/Encodings/MFM.cpp b/Storage/Disk/Encodings/MFM.cpp index 102815d07..e8cf95179 100644 --- a/Storage/Disk/Encodings/MFM.cpp +++ b/Storage/Disk/Encodings/MFM.cpp @@ -413,6 +413,7 @@ std::shared_ptr Parser::get_sector(uint8_t sect while(1) { std::shared_ptr next_sector = get_next_sector(); + if(!next_sector) continue; if(next_sector->sector == first_sector->sector) return nullptr; if(next_sector->sector == sector) return next_sector; } From f88f3c65e9b3746a7c7720cc6798be7dbc8d05ce Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 30 Dec 2016 14:21:36 -0500 Subject: [PATCH 42/52] Removed duplicated newline. --- Storage/Disk/Formats/SSD.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/Storage/Disk/Formats/SSD.cpp b/Storage/Disk/Formats/SSD.cpp index b7977df58..006dfc642 100644 --- a/Storage/Disk/Formats/SSD.cpp +++ b/Storage/Disk/Formats/SSD.cpp @@ -35,7 +35,6 @@ SSD::~SSD() flush_updates(); } - unsigned int SSD::get_head_position_count() { return track_count_; From 71dbd78cf20c88385482ebd5d4680d1ade9bc982 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 30 Dec 2016 14:23:26 -0500 Subject: [PATCH 43/52] If asynchronous background processing is to occur on tracks then, given that they inherently have state, they'll need to be copyable, and ideally 'cheaply' (though it's not too great a priority). So started implementing appropriate copy constructors. Also introduced an extra level of indirection to `PCMSegmentEventSource` so that it can copy itself without copying the underlying `PCMSegment`, which is 95% of the heft of a track in all currently-implemented cases. --- Storage/Disk/PCMSegment.cpp | 41 ++++++++++++++++++++++--------------- Storage/Disk/PCMSegment.hpp | 10 +++++++-- Storage/Disk/PCMTrack.cpp | 5 +++++ Storage/Disk/PCMTrack.hpp | 9 ++++++-- 4 files changed, 45 insertions(+), 20 deletions(-) diff --git a/Storage/Disk/PCMSegment.cpp b/Storage/Disk/PCMSegment.cpp index ed4335e8e..cdde8024d 100644 --- a/Storage/Disk/PCMSegment.cpp +++ b/Storage/Disk/PCMSegment.cpp @@ -11,24 +11,33 @@ using namespace Storage::Disk; PCMSegmentEventSource::PCMSegmentEventSource(const PCMSegment &segment) : - segment_(segment) + segment_(new PCMSegment) { + *segment_ = segment; + // add an extra bit of storage at the bottom if one is going to be needed; // events returned are going to be in integral multiples of the length of a bit // other than the very first and very last which will include a half bit length - if(segment_.length_of_a_bit.length&1) + if(segment_->length_of_a_bit.length&1) { - segment_.length_of_a_bit.length <<= 1; - segment_.length_of_a_bit.clock_rate <<= 1; + segment_->length_of_a_bit.length <<= 1; + segment_->length_of_a_bit.clock_rate <<= 1; } // load up the clock rate once only - next_event_.length.clock_rate = segment_.length_of_a_bit.clock_rate; + next_event_.length.clock_rate = segment_->length_of_a_bit.clock_rate; // set initial conditions reset(); } +PCMSegmentEventSource::PCMSegmentEventSource(const PCMSegmentEventSource &original) +{ + // share underlying data with the original + segment_ = original.segment_; + reset(); +} + void PCMSegmentEventSource::reset() { // start with the first bit to be considered the zeroth, and assume that it'll be @@ -45,15 +54,15 @@ Storage::Disk::Track::Event PCMSegmentEventSource::get_next_event() // if starting from the beginning, pull half a bit backward, as if the initial bit // is set, it should be in the centre of its window - next_event_.length.length = bit_pointer_ ? 0 : -(segment_.length_of_a_bit.length >> 1); + next_event_.length.length = bit_pointer_ ? 0 : -(segment_->length_of_a_bit.length >> 1); // search for the next bit that is set, if any - const uint8_t *segment_data = segment_.data.data(); - while(bit_pointer_ < segment_.number_of_bits) + const uint8_t *segment_data = segment_->data.data(); + while(bit_pointer_ < segment_->number_of_bits) { int bit = segment_data[bit_pointer_ >> 3] & (0x80 >> (bit_pointer_&7)); bit_pointer_++; // so this always points one beyond the most recent bit returned - next_event_.length.length += segment_.length_of_a_bit.length; + next_event_.length.length += segment_->length_of_a_bit.length; // if this bit is set, return the event if(bit) return next_event_; @@ -66,9 +75,9 @@ Storage::Disk::Track::Event PCMSegmentEventSource::get_next_event() // allow an extra half bit's length to run from the position of the potential final transition // event to the end of the segment. Otherwise don't allow any extra time, as it's already // been consumed - if(initial_bit_pointer <= segment_.number_of_bits) + if(initial_bit_pointer <= segment_->number_of_bits) { - next_event_.length.length += (segment_.length_of_a_bit.length >> 1); + next_event_.length.length += (segment_->length_of_a_bit.length >> 1); bit_pointer_++; } return next_event_; @@ -76,7 +85,7 @@ Storage::Disk::Track::Event PCMSegmentEventSource::get_next_event() Storage::Time PCMSegmentEventSource::get_length() { - return segment_.length_of_a_bit * segment_.number_of_bits; + return segment_->length_of_a_bit * segment_->number_of_bits; } Storage::Time PCMSegmentEventSource::seek_to(const Time &time_from_start) @@ -86,7 +95,7 @@ Storage::Time PCMSegmentEventSource::seek_to(const Time &time_from_start) if(time_from_start >= length) { next_event_.type = Track::Event::IndexHole; - bit_pointer_ = segment_.number_of_bits+1; + bit_pointer_ = segment_->number_of_bits+1; return length; } @@ -94,7 +103,7 @@ Storage::Time PCMSegmentEventSource::seek_to(const Time &time_from_start) next_event_.type = Track::Event::FluxTransition; // test for requested time being before the first bit - Time half_bit_length = segment_.length_of_a_bit; + Time half_bit_length = segment_->length_of_a_bit; half_bit_length.length >>= 1; if(time_from_start < half_bit_length) { @@ -107,8 +116,8 @@ Storage::Time PCMSegmentEventSource::seek_to(const Time &time_from_start) // bit_pointer_ always records _the next bit_ that might trigger an event, // so should be one beyond the one reached by a seek. Time relative_time = time_from_start - half_bit_length; - bit_pointer_ = 1 + (relative_time / segment_.length_of_a_bit).get_unsigned_int(); + bit_pointer_ = 1 + (relative_time / segment_->length_of_a_bit).get_unsigned_int(); // map up to the correct amount of time - return half_bit_length + segment_.length_of_a_bit * (unsigned int)(bit_pointer_ - 1); + return half_bit_length + segment_->length_of_a_bit * (unsigned int)(bit_pointer_ - 1); } diff --git a/Storage/Disk/PCMSegment.hpp b/Storage/Disk/PCMSegment.hpp index 4abc84293..c753d194c 100644 --- a/Storage/Disk/PCMSegment.hpp +++ b/Storage/Disk/PCMSegment.hpp @@ -42,7 +42,13 @@ class PCMSegmentEventSource { Constructs a @c PCMSegmentEventSource that will derive events from @c segment. The event source is initially @c reset. */ - PCMSegmentEventSource(const PCMSegment &segment); + PCMSegmentEventSource(const PCMSegment &); + + /*! + Copy constructor; produces a segment event source with the same underlying segment + but a unique pointer into it. + */ + PCMSegmentEventSource(const PCMSegmentEventSource &); /*! @returns the next event that will occur in this event stream. @@ -69,7 +75,7 @@ class PCMSegmentEventSource { Time get_length(); private: - PCMSegment segment_; + std::shared_ptr segment_; size_t bit_pointer_; Track::Event next_event_; }; diff --git a/Storage/Disk/PCMTrack.cpp b/Storage/Disk/PCMTrack.cpp index f89967ef4..0e1b401f0 100644 --- a/Storage/Disk/PCMTrack.cpp +++ b/Storage/Disk/PCMTrack.cpp @@ -47,6 +47,11 @@ PCMTrack::PCMTrack(const PCMSegment &segment) : PCMTrack() segment_event_sources_.emplace_back(length_adjusted_segment); } +PCMTrack::PCMTrack(const PCMTrack &original) : PCMTrack() +{ + segment_event_sources_ = original.segment_event_sources_; +} + Track::Event PCMTrack::get_next_event() { // ask the current segment for a new event diff --git a/Storage/Disk/PCMTrack.hpp b/Storage/Disk/PCMTrack.hpp index 255bed80f..ce11d7125 100644 --- a/Storage/Disk/PCMTrack.hpp +++ b/Storage/Disk/PCMTrack.hpp @@ -28,13 +28,18 @@ class PCMTrack: public Track { /*! Creates a @c PCMTrack consisting of multiple segments of data, permitting multiple clock rates. */ - PCMTrack(const std::vector &segments); + PCMTrack(const std::vector &); /*! Creates a @c PCMTrack consisting of a single continuous run of data, implying a constant clock rate. The segment's @c length_of_a_bit will be ignored and therefore need not be filled in. */ - PCMTrack(const PCMSegment &segment); + PCMTrack(const PCMSegment &); + + /*! + Copy constructor; required for Tracks in order to support modifiable disks. + */ + PCMTrack(const PCMTrack &); // as per @c Track Event get_next_event(); From 63ff5165a4046790e7f247804f0008c21285b84f Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 30 Dec 2016 17:25:39 -0500 Subject: [PATCH 44/52] After a quick bit of reading, discovered the virtual copy constructor pattern really is only a convention in C++, and conformed to it. Which hopefully gives copyable tracks. --- Storage/Disk/Disk.hpp | 5 +++++ Storage/Disk/PCMPatchedTrack.cpp | 12 ++++++++++++ Storage/Disk/PCMPatchedTrack.hpp | 6 ++++++ Storage/Disk/PCMTrack.cpp | 5 +++++ Storage/Disk/PCMTrack.hpp | 1 + 5 files changed, 29 insertions(+) diff --git a/Storage/Disk/Disk.hpp b/Storage/Disk/Disk.hpp index b23081a04..f574f2c28 100644 --- a/Storage/Disk/Disk.hpp +++ b/Storage/Disk/Disk.hpp @@ -53,6 +53,11 @@ class Track { @returns the time jumped to. */ virtual Time seek_to(const Time &time_since_index_hole) = 0; + + /*! + The virtual copy constructor pattern; returns a copy of the Track. + */ + virtual Track *clone() = 0; }; /*! diff --git a/Storage/Disk/PCMPatchedTrack.cpp b/Storage/Disk/PCMPatchedTrack.cpp index 4356912c5..05ea73d1b 100644 --- a/Storage/Disk/PCMPatchedTrack.cpp +++ b/Storage/Disk/PCMPatchedTrack.cpp @@ -20,6 +20,18 @@ PCMPatchedTrack::PCMPatchedTrack(std::shared_ptr underlying_track) : underlying_track_->seek_to(zero); } +PCMPatchedTrack::PCMPatchedTrack(const PCMPatchedTrack &original) +{ + underlying_track_.reset(original.underlying_track_->clone()); + periods_ = original.periods_; + active_period_ = periods_.begin(); +} + +Track *PCMPatchedTrack::clone() +{ + return new PCMPatchedTrack(*this); +} + void PCMPatchedTrack::add_segment(const Time &start_time, const PCMSegment &segment) { std::shared_ptr event_source(new PCMSegmentEventSource(segment)); diff --git a/Storage/Disk/PCMPatchedTrack.hpp b/Storage/Disk/PCMPatchedTrack.hpp index 2ea8ffd04..d4cad19ed 100644 --- a/Storage/Disk/PCMPatchedTrack.hpp +++ b/Storage/Disk/PCMPatchedTrack.hpp @@ -26,6 +26,11 @@ class PCMPatchedTrack: public Track { */ PCMPatchedTrack(std::shared_ptr underlying_track); + /*! + Copy constructor, for Track. + */ + PCMPatchedTrack(const PCMPatchedTrack &); + /*! Replaces whatever is currently on the track from @c start_position to @c start_position + segment length with the contents of @c segment. @@ -35,6 +40,7 @@ class PCMPatchedTrack: public Track { // To satisfy Storage::Disk::Track Event get_next_event(); Time seek_to(const Time &time_since_index_hole); + Track *clone(); private: std::shared_ptr underlying_track_; diff --git a/Storage/Disk/PCMTrack.cpp b/Storage/Disk/PCMTrack.cpp index 0e1b401f0..9a8067f31 100644 --- a/Storage/Disk/PCMTrack.cpp +++ b/Storage/Disk/PCMTrack.cpp @@ -52,6 +52,11 @@ PCMTrack::PCMTrack(const PCMTrack &original) : PCMTrack() segment_event_sources_ = original.segment_event_sources_; } +Track *PCMTrack::clone() +{ + return new PCMTrack(*this); +} + Track::Event PCMTrack::get_next_event() { // ask the current segment for a new event diff --git a/Storage/Disk/PCMTrack.hpp b/Storage/Disk/PCMTrack.hpp index ce11d7125..ad1d7093a 100644 --- a/Storage/Disk/PCMTrack.hpp +++ b/Storage/Disk/PCMTrack.hpp @@ -44,6 +44,7 @@ class PCMTrack: public Track { // as per @c Track Event get_next_event(); Time seek_to(const Time &time_since_index_hole); + Track *clone(); private: // storage for the segments that describe this track From 81a3cbac4523f8b3885b5d4409fd8f00b143e9b5 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 30 Dec 2016 17:26:44 -0500 Subject: [PATCH 45/52] Ensured a copy is passed for writing back rather than the original. --- Storage/Disk/Disk.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Storage/Disk/Disk.cpp b/Storage/Disk/Disk.cpp index 6938014e7..fe7d1cea4 100644 --- a/Storage/Disk/Disk.cpp +++ b/Storage/Disk/Disk.cpp @@ -23,8 +23,9 @@ void Disk::set_track_at_position(unsigned int head, unsigned int position, const cached_tracks_[address] = track; if(!update_queue_) update_queue_.reset(new Concurrency::AsyncTaskQueue); - update_queue_->enqueue([this, head, position, track] { - store_updated_track_at_position(head, position, track, file_access_mutex_); + std::shared_ptr track_copy(track->clone()); + update_queue_->enqueue([this, head, position, track_copy] { + store_updated_track_at_position(head, position, track_copy, file_access_mutex_); }); } From e5cc77f22d1e627414ef19a02350da1f1818a30e Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 30 Dec 2016 17:29:51 -0500 Subject: [PATCH 46/52] Added an extra sanity check. --- StaticAnalyser/Acorn/Disk.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/StaticAnalyser/Acorn/Disk.cpp b/StaticAnalyser/Acorn/Disk.cpp index c7cf4b882..00c77328e 100644 --- a/StaticAnalyser/Acorn/Disk.cpp +++ b/StaticAnalyser/Acorn/Disk.cpp @@ -28,6 +28,7 @@ std::unique_ptr StaticAnalyser::Acorn::GetDFSCatalogue(const std::sha uint8_t final_file_offset = details->data[5]; if(final_file_offset&7) return nullptr; + if(final_file_offset < 8) return nullptr; char disk_name[13]; snprintf(disk_name, 13, "%.8s%.4s", &names->data[0], &details->data[0]); From 5d635568700e2aae1626c64eb720099e3054e112 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 30 Dec 2016 17:39:52 -0500 Subject: [PATCH 47/52] `Period`s need a custom copy constructor too, if they're going to avoid sharing an event_source. --- Storage/Disk/PCMPatchedTrack.cpp | 6 ++++++ Storage/Disk/PCMPatchedTrack.hpp | 1 + 2 files changed, 7 insertions(+) diff --git a/Storage/Disk/PCMPatchedTrack.cpp b/Storage/Disk/PCMPatchedTrack.cpp index 05ea73d1b..7befd23c1 100644 --- a/Storage/Disk/PCMPatchedTrack.cpp +++ b/Storage/Disk/PCMPatchedTrack.cpp @@ -224,6 +224,12 @@ Storage::Time PCMPatchedTrack::seek_to(const Time &time_since_index_hole) return current_time_; } +PCMPatchedTrack::Period::Period(const Period &original) : + start_time(original.start_time), end_time(original.end_time), segment_start_time(original.segment_start_time) +{ + if(original.event_source) event_source.reset(new PCMSegmentEventSource(*original.event_source)); +} + void PCMPatchedTrack::Period::push_start_to_time(const Storage::Time &new_start_time) { segment_start_time += new_start_time - start_time; diff --git a/Storage/Disk/PCMPatchedTrack.hpp b/Storage/Disk/PCMPatchedTrack.hpp index d4cad19ed..23e7bd3a1 100644 --- a/Storage/Disk/PCMPatchedTrack.hpp +++ b/Storage/Disk/PCMPatchedTrack.hpp @@ -55,6 +55,7 @@ class PCMPatchedTrack: public Track { Period(const Time &start_time, const Time &end_time, const Time &segment_start_time, std::shared_ptr event_source) : start_time(start_time), end_time(end_time), segment_start_time(segment_start_time), event_source(event_source) {} + Period(const Period &); }; std::vector periods_; std::vector::iterator active_period_; From d09e7ac1e82fcacc41dda2e2ed2a34eaebdb78c3 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 30 Dec 2016 17:44:35 -0500 Subject: [PATCH 48/52] Made an attempt at reacting appropriately if the very first thing that looks like a sector doesn't pan out. --- Storage/Disk/Encodings/MFM.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Storage/Disk/Encodings/MFM.cpp b/Storage/Disk/Encodings/MFM.cpp index e8cf95179..c0086c071 100644 --- a/Storage/Disk/Encodings/MFM.cpp +++ b/Storage/Disk/Encodings/MFM.cpp @@ -406,7 +406,8 @@ std::shared_ptr Parser::get_next_sector() std::shared_ptr Parser::get_sector(uint8_t sector) { - std::shared_ptr first_sector = get_next_sector(); + std::shared_ptr first_sector; + while(!first_sector && index_count_ < 2) first_sector = get_next_sector(); if(!first_sector) return first_sector; if(first_sector->sector == sector) return first_sector; From c740d9655ab1714eb95dc6b2840bfb905790b080 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 30 Dec 2016 17:55:06 -0500 Subject: [PATCH 49/52] Fixed: index_count_ may have been left high by a previous call; reset it just in case. --- Storage/Disk/Encodings/MFM.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/Storage/Disk/Encodings/MFM.cpp b/Storage/Disk/Encodings/MFM.cpp index c0086c071..d1740baeb 100644 --- a/Storage/Disk/Encodings/MFM.cpp +++ b/Storage/Disk/Encodings/MFM.cpp @@ -407,6 +407,7 @@ std::shared_ptr Parser::get_next_sector() std::shared_ptr Parser::get_sector(uint8_t sector) { std::shared_ptr first_sector; + index_count_ = 0; while(!first_sector && index_count_ < 2) first_sector = get_next_sector(); if(!first_sector) return first_sector; if(first_sector->sector == sector) return first_sector; From c85450648fbf23fae6e427fe9cf28e77367fe811 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 30 Dec 2016 17:55:46 -0500 Subject: [PATCH 50/52] Fix: make sure copies have proper event lengths. Also made it much clearer what's going on with the initial copy to the heap. --- Storage/Disk/PCMSegment.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Storage/Disk/PCMSegment.cpp b/Storage/Disk/PCMSegment.cpp index cdde8024d..59953ee6a 100644 --- a/Storage/Disk/PCMSegment.cpp +++ b/Storage/Disk/PCMSegment.cpp @@ -11,10 +11,8 @@ using namespace Storage::Disk; PCMSegmentEventSource::PCMSegmentEventSource(const PCMSegment &segment) : - segment_(new PCMSegment) + segment_(new PCMSegment(segment)) { - *segment_ = segment; - // add an extra bit of storage at the bottom if one is going to be needed; // events returned are going to be in integral multiples of the length of a bit // other than the very first and very last which will include a half bit length @@ -35,6 +33,9 @@ PCMSegmentEventSource::PCMSegmentEventSource(const PCMSegmentEventSource &origin { // share underlying data with the original segment_ = original.segment_; + + // load up the clock rate and set initial conditions + next_event_.length.clock_rate = segment_->length_of_a_bit.clock_rate; reset(); } From 07dacff42d47ec139710db6a8b4cfda548ab8f2d Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 30 Dec 2016 18:03:30 -0500 Subject: [PATCH 51/52] Added writing for Acorn ADF disks, plus appropriate TODOs in both similar bits of boilerplate. --- Storage/Disk/Formats/AcornADF.cpp | 50 ++++++++++++++++++------------- Storage/Disk/Formats/AcornADF.hpp | 3 ++ Storage/Disk/Formats/SSD.cpp | 2 ++ Storage/Disk/Formats/SSD.hpp | 1 + 4 files changed, 36 insertions(+), 20 deletions(-) diff --git a/Storage/Disk/Formats/AcornADF.cpp b/Storage/Disk/Formats/AcornADF.cpp index 9615139f9..28b319067 100644 --- a/Storage/Disk/Formats/AcornADF.cpp +++ b/Storage/Disk/Formats/AcornADF.cpp @@ -40,25 +40,6 @@ AcornADF::AcornADF(const char *file_name) : AcornADF::~AcornADF() { flush_updates(); -// if(get_is_modified()) -// { -// for(unsigned int head = 0; head < get_head_count(); head++) -// { -// for(unsigned int track = 0; track < get_head_position_count(); track++) -// { -// std::shared_ptr modified_track = get_modified_track_at_position(head, track); -// if(modified_track) -// { -// Storage::Encodings::MFM::Parser parser(true, modified_track); -// for(unsigned int c = 0; c < sectors_per_track; c++) -// { -// std::shared_ptr sector = parser.get_sector((uint8_t)track, (uint8_t)c); -// printf("Sector %d: %p\n", c, sector.get()); -// } -// } -// } -// } -// } } unsigned int AcornADF::get_head_position_count() @@ -76,12 +57,17 @@ bool AcornADF::get_is_read_only() return is_read_only_; } +long AcornADF::get_file_offset_for_position(unsigned int head, unsigned int position) +{ + return (position * 1 + head) * bytes_per_sector * sectors_per_track; +} + std::shared_ptr AcornADF::get_uncached_track_at_position(unsigned int head, unsigned int position) { std::shared_ptr track; if(head >= 2) return track; - long file_offset = (position * 1 + head) * bytes_per_sector * sectors_per_track; + long file_offset = get_file_offset_for_position(head, position); fseek(file_, file_offset, SEEK_SET); std::vector sectors; @@ -104,3 +90,27 @@ std::shared_ptr AcornADF::get_uncached_track_at_position(unsigned int hea return track; } + +void AcornADF::store_updated_track_at_position(unsigned int head, unsigned int position, const std::shared_ptr &track, std::mutex &file_access_mutex) +{ + std::vector parsed_track; + Storage::Encodings::MFM::Parser parser(true, track); + for(unsigned int c = 0; c < sectors_per_track; c++) + { + std::shared_ptr sector = parser.get_sector((uint8_t)position, (uint8_t)c); + if(sector) + { + parsed_track.insert(parsed_track.end(), sector->data.begin(), sector->data.end()); + } + else + { + // TODO: what's correct here? Warn the user that whatever has been written to the disk, + // it can no longer be stored as an SSD? If so, warn them by what route? + parsed_track.resize(parsed_track.size() + bytes_per_sector); + } + } + + std::lock_guard lock_guard(file_access_mutex); + fseek(file_, get_file_offset_for_position(head, position), SEEK_SET); + fwrite(parsed_track.data(), 1, parsed_track.size(), file_); +} diff --git a/Storage/Disk/Formats/AcornADF.hpp b/Storage/Disk/Formats/AcornADF.hpp index ea996303f..0100a4d2c 100644 --- a/Storage/Disk/Formats/AcornADF.hpp +++ b/Storage/Disk/Formats/AcornADF.hpp @@ -37,8 +37,11 @@ class AcornADF: public Disk, public Storage::FileHolder { unsigned int get_head_position_count(); unsigned int get_head_count(); bool get_is_read_only(); + private: + void store_updated_track_at_position(unsigned int head, unsigned int position, const std::shared_ptr &track, std::mutex &file_access_mutex); std::shared_ptr get_uncached_track_at_position(unsigned int head, unsigned int position); + long get_file_offset_for_position(unsigned int head, unsigned int position); }; } diff --git a/Storage/Disk/Formats/SSD.cpp b/Storage/Disk/Formats/SSD.cpp index 006dfc642..b91c7dcbd 100644 --- a/Storage/Disk/Formats/SSD.cpp +++ b/Storage/Disk/Formats/SSD.cpp @@ -99,6 +99,8 @@ void SSD::store_updated_track_at_position(unsigned int head, unsigned int positi } else { + // TODO: what's correct here? Warn the user that whatever has been written to the disk, + // it can no longer be stored as an SSD? If so, warn them by what route? parsed_track.resize(parsed_track.size() + 256); } } diff --git a/Storage/Disk/Formats/SSD.hpp b/Storage/Disk/Formats/SSD.hpp index 0e806a147..a26432c10 100644 --- a/Storage/Disk/Formats/SSD.hpp +++ b/Storage/Disk/Formats/SSD.hpp @@ -42,6 +42,7 @@ class SSD: public Disk, public Storage::FileHolder { void store_updated_track_at_position(unsigned int head, unsigned int position, const std::shared_ptr &track, std::mutex &file_access_mutex); std::shared_ptr get_uncached_track_at_position(unsigned int head, unsigned int position); long get_file_offset_for_position(unsigned int head, unsigned int position); + unsigned int head_count_; unsigned int track_count_; }; From 3b29e6a47338ff75d7fdd207409e96601b661603 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 30 Dec 2016 18:08:12 -0500 Subject: [PATCH 52/52] Ensured SSD and ADFs are grown if required. --- Storage/Disk/Formats/AcornADF.cpp | 4 +++- Storage/Disk/Formats/SSD.cpp | 4 +++- Storage/FileHolder.cpp | 13 +++++++++++++ Storage/FileHolder.hpp | 6 ++++++ 4 files changed, 25 insertions(+), 2 deletions(-) diff --git a/Storage/Disk/Formats/AcornADF.cpp b/Storage/Disk/Formats/AcornADF.cpp index 28b319067..1568fbb86 100644 --- a/Storage/Disk/Formats/AcornADF.cpp +++ b/Storage/Disk/Formats/AcornADF.cpp @@ -111,6 +111,8 @@ void AcornADF::store_updated_track_at_position(unsigned int head, unsigned int p } std::lock_guard lock_guard(file_access_mutex); - fseek(file_, get_file_offset_for_position(head, position), SEEK_SET); + long file_offset = get_file_offset_for_position(head, position); + ensure_file_is_at_least_length(file_offset); + fseek(file_, file_offset, SEEK_SET); fwrite(parsed_track.data(), 1, parsed_track.size(), file_); } diff --git a/Storage/Disk/Formats/SSD.cpp b/Storage/Disk/Formats/SSD.cpp index b91c7dcbd..0c7f23984 100644 --- a/Storage/Disk/Formats/SSD.cpp +++ b/Storage/Disk/Formats/SSD.cpp @@ -106,6 +106,8 @@ void SSD::store_updated_track_at_position(unsigned int head, unsigned int positi } std::lock_guard lock_guard(file_access_mutex); - fseek(file_, get_file_offset_for_position(head, position), SEEK_SET); + long file_offset = get_file_offset_for_position(head, position); + ensure_file_is_at_least_length(file_offset); + fseek(file_, file_offset, SEEK_SET); fwrite(parsed_track.data(), 1, parsed_track.size(), file_); } diff --git a/Storage/FileHolder.cpp b/Storage/FileHolder.cpp index 6271a79cd..1395da998 100644 --- a/Storage/FileHolder.cpp +++ b/Storage/FileHolder.cpp @@ -85,3 +85,16 @@ uint16_t FileHolder::fgetc16be() return result; } + +void FileHolder::ensure_file_is_at_least_length(long length) +{ + fseek(file_, 0, SEEK_END); + long bytes_to_write = length - ftell(file_); + if(bytes_to_write > 0) + { + uint8_t *empty = new uint8_t[bytes_to_write]; + memset(empty, 0, bytes_to_write); + fwrite(empty, sizeof(uint8_t), (size_t)bytes_to_write, file_); + delete[] empty; + } +} diff --git a/Storage/FileHolder.hpp b/Storage/FileHolder.hpp index bf1029102..458927608 100644 --- a/Storage/FileHolder.hpp +++ b/Storage/FileHolder.hpp @@ -65,6 +65,12 @@ class FileHolder { */ uint16_t fgetc16be(); + /*! + Ensures the file is at least @c length bytes long, appending 0s until it is + if necessary. + */ + void ensure_file_is_at_least_length(long length); + FILE *file_; struct stat file_stats_; bool is_read_only_;