diff --git a/Components/1770/1770.cpp b/Components/1770/1770.cpp index 00aa5711f..1a0b393ea 100644 --- a/Components/1770/1770.cpp +++ b/Components/1770/1770.cpp @@ -481,7 +481,7 @@ void WD1770::posit_event(int new_event_type) { status.data_request = true; }); distance_into_section_++; - if(distance_into_section_ == 128 << header_[3]) { + if(distance_into_section_ == 128 << (header_[3]&3)) { distance_into_section_ = 0; goto type2_check_crc; } @@ -564,7 +564,7 @@ void WD1770::posit_event(int new_event_type) { */ write_byte(data_); distance_into_section_++; - if(distance_into_section_ == 128 << header_[3]) { + if(distance_into_section_ == 128 << (header_[3]&3)) { goto type2_write_crc; } diff --git a/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme b/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme index 1465a4f62..47f9c7286 100644 --- a/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme +++ b/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme @@ -67,7 +67,7 @@ contents; + }; + /// @returns The byte stream this sector address would produce if a WD read track command were to observe it. - std::vector get_track_address_image() const { - return track_encoding(address.begin(), address.begin() + 4, {0xa1, 0xa1, 0xfe}); + std::vector get_track_address_fragments() const { + return track_fragments(address.begin(), address.begin() + 4, {0xa1, 0xa1, 0xfe}); } /// @returns The byte stream this sector data would produce if a WD read track command were to observe it. - std::vector get_track_data_image() const { - return track_encoding(contents.begin(), contents.end(), {0xa1, 0xa1, 0xfb}); + std::vector get_track_data_fragments() const { + return track_fragments(contents.begin(), contents.end(), {0xa1, 0xa1, 0xfb}); + } + + /*! + Acts like std::search except that it tries to find a start location from which all of the members of @c fragments + can be found in successive order with no more than a 'permissible' amount of gap between them. + + Where 'permissible' is derived empirically from trial and error; in practice it's a measure of the number of bytes + a WD may produce when it has encountered a false sync, and I don't have documentation on that. So it's + derived from in-practice testing of STXs (which, hopefully, contain an accurate copy of what a WD would do, + so are themselves possibly a way to research that). + */ + template static Iterator find_fragments(Iterator begin, Iterator end, const std::vector &fragments) { + while(true) { + // To match the fragments, they must all be found, in order, with at most two bytes of gap. + auto this_begin = begin; + std::vector::const_iterator first_location = end; + bool is_found = true; + bool is_first = true; + for(auto fragment: fragments) { + auto location = std::search(this_begin, end, fragment.contents.begin(), fragment.contents.end()); + + // If fragment wasn't found at all, it's never going to be found. So game over. + if(location == end) { + return location; + } + + // Otherwise, either mark + if(is_first) { + first_location = location; + } else if(location > this_begin + 5*fragment.prior_syncs) { + is_found = false; + break; + } + + is_first = false; + this_begin = location + ssize_t(fragment.contents.size()); + } + + if(is_found) { + return first_location; + } + + // TODO: can I assume more than this? + ++begin; + } + return end; } private: /// @returns The effect of encoding @c prefix followed by the bytes from @c begin to @c end as MFM data and then decoding them as if - /// observed by a WD read track command. - template static std::vector track_encoding(T begin, T end, std::initializer_list prefix) { - std::vector result; + /// observed by a WD read track command, split into fragments separated by any instances of false sync — since it's still unclear to me exactly what + /// a WD should put out in those instances. + template static std::vector track_fragments(T begin, T end, std::initializer_list prefix) { + std::vector result; result.reserve(size_t(end - begin) + prefix.size()); PCMSegment segment; @@ -79,17 +131,44 @@ class TrackConstructor { ++begin; } - // Decode, obeying false syncs. + // Decode, starting a new segment upon any false sync since I don't have good documentation + // presently on exactly how a WD should react to those. using Shifter = Storage::Encodings::MFM::Shifter; Shifter shifter; shifter.set_should_obey_syncs(true); + shifter.set_is_double_density(true); + + result.emplace_back(); // Add whatever comes from the track. + int ignore_count = 0; for(auto bit: segment.data) { shifter.add_input_bit(int(bit)); - if(shifter.get_token() != Shifter::None) { - result.push_back(shifter.get_byte()); + const auto token = shifter.get_token(); + if(token != Shifter::None) { + if(ignore_count) { + --ignore_count; + continue; + } + + // If anything other than a byte is encountered, + // skip it and the next thing to be reported, + // beginning a new fragment. + if(token != Shifter::Token::Byte) { + ignore_count = 1; + + if(!result.back().contents.empty()) { + result.emplace_back(); + } else { + ++result.back().prior_syncs; + } + + continue; + } + + // This was an ordinary byte, retain it. + result.back().contents.push_back(shifter.get_byte()); } } @@ -127,8 +206,7 @@ class TrackConstructor { // To reconcile the list of sectors with the WD get track-style track image, // use sector bodies as definitive and refer to the track image for in-fill. auto track_position = track_data_.begin(); - const auto address_mark = {0xa1, 0xa1, 0xfe}; - const auto track_mark = {0xa1, 0xa1, 0xfb}; + const auto sync_mark = {0xa1, 0xa1}; struct Location { enum Type { Address, Data @@ -142,35 +220,39 @@ class TrackConstructor { for(const auto §or: sectors_) { { // Find out what the address would look like, if found in a read track. - const auto track_address = sector.get_track_address_image(); + const auto address_fragments = sector.get_track_address_fragments(); // Try to locate the header within the track image; if it can't be found then settle for // the next thing that looks like a header of any sort. - auto address_position = std::search(track_position, track_data_.end(), track_address.begin(), track_address.end()); + auto address_position = TrackConstructor::Sector::find_fragments(track_position, track_data_.end(), address_fragments); if(address_position == track_data_.end()) { - address_position = std::search(track_position, track_data_.end(), address_mark.begin(), address_mark.end()); + address_position = std::search(track_position, track_data_.end(), sync_mark.begin(), sync_mark.end()); } - // Stop now if there's nowhere obvious to put this sector. - if(address_position == track_data_.end()) break; - locations.emplace_back(Location::Address, address_position, sector); + // Place this address only if somewhere to put it was found. + if(address_position != track_data_.end()) { + locations.emplace_back(Location::Address, address_position, sector); - // Advance the track position. - track_position = address_position; + // Advance the track position. + track_position = address_position + 6; + } } // Do much the same thing for the data, if it exists. if(!(sector.status & 0x10)) { - const auto track_data = sector.get_track_data_image(); + const auto data_fragments = sector.get_track_data_fragments(); - auto data_position = std::search(track_position, track_data_.end(), track_data.begin(), track_data.end()); + auto data_position = TrackConstructor::Sector::find_fragments(track_position, track_data_.end(), data_fragments); if(data_position == track_data_.end()) { - data_position = std::search(track_position, track_data_.end(), track_mark.begin(), track_mark.end()); + data_position = std::search(track_position, track_data_.end(), sync_mark.begin(), sync_mark.end()); + } + if(data_position == track_data_.end()) { + // Desperation: guess from the given offset. + data_position = track_data_.begin() + (sector.bit_position / 16); } - if(data_position == track_data_.end()) break; locations.emplace_back(Location::Data, data_position, sector); - track_position = data_position; + track_position = data_position + sector.data_size(); } } @@ -184,8 +266,10 @@ class TrackConstructor { auto location = locations.begin(); track_position = track_data_.begin(); while(location != locations.end()) { +// assert(location->position >= track_position && location->position < track_data_.end()); + // Advance to location.position. - while(track_position != location->position) { + while(track_position < location->position) { encoder->add_byte(*track_position); ++track_position; } diff --git a/Storage/Disk/Encodings/MFM/Shifter.cpp b/Storage/Disk/Encodings/MFM/Shifter.cpp index b68c6f210..efe428b73 100644 --- a/Storage/Disk/Encodings/MFM/Shifter.cpp +++ b/Storage/Disk/Encodings/MFM/Shifter.cpp @@ -25,7 +25,7 @@ void Shifter::set_should_obey_syncs(bool should_obey_syncs) { void Shifter::add_input_bit(int value) { shift_register_ = (shift_register_ << 1) | static_cast(value); - bits_since_token_++; + ++bits_since_token_; token_ = Token::None; if(should_obey_syncs_) { @@ -57,6 +57,20 @@ void Shifter::add_input_bit(int value) { } else { switch(shift_register_ & 0xffff) { case Storage::Encodings::MFM::MFMIndexSync: + // This models a bit of slightly weird WD behaviour; + // if an index sync was detected where it wasn't expected, + // the WD will resync but also may return the clock bits + // rather than data bits as the next byte read, depending + // on framing. + // + // TODO: make this optional, if ever a Shifter with + // well-defined non-WD behaviour is needed. + // + // TODO: Verify WD behaviour. + if(bits_since_token_&1) { + shift_register_ >>= 1; + } + bits_since_token_ = 0; is_awaiting_marker_value_ = true; diff --git a/Storage/Disk/Encodings/MFM/Shifter.hpp b/Storage/Disk/Encodings/MFM/Shifter.hpp index 46bd252f6..35234c1ca 100644 --- a/Storage/Disk/Encodings/MFM/Shifter.hpp +++ b/Storage/Disk/Encodings/MFM/Shifter.hpp @@ -27,6 +27,10 @@ namespace MFM { It will ordinarily honour sync patterns; that should be turned off when within a sector because false syncs can occur. See @c set_should_obey_syncs. + It aims to implement the same behaviour as WD177x-series controllers when + detecting a false sync — the received byte value will be either a 0xc1 or 0x14, + depending on phase. + Bits should be fed in with @c add_input_bit. The current output token can be read with @c get_token. It will usually be None but @@ -62,14 +66,14 @@ class Shifter { } private: - // Bit stream input state + // Bit stream input state. int bits_since_token_ = 0; unsigned int shift_register_ = 0; bool is_awaiting_marker_value_ = false; bool should_obey_syncs_ = true; Token token_ = None; - // input configuration + // Input configuration. bool is_double_density_ = false; std::unique_ptr owned_crc_generator_;