Made steps towards proper CRC generation. Am currently comparing against Oric disk images, as — amongst other things — they include precomputed CRCs.

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_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]);
+				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)

Machines/Oric/Microdisc.cpp

@@ -38,47 +38,60 @@ void Microdisc::set_disk(std::shared_ptr<Storage::Disk::Disk> disk, int drive)
 
 void Microdisc::set_control_register(uint8_t control)
 {
-	printf("control: %d%d%d%d%d%d%d%d\n",
+//	printf("control: %d%d%d%d%d%d%d%d\n",
-		(control >> 7)&1,
+//		(control >> 7)&1,
-		(control >> 6)&1,
+//		(control >> 6)&1,
-		(control >> 5)&1,
+//		(control >> 5)&1,
-		(control >> 4)&1,
+//		(control >> 4)&1,
-		(control >> 3)&1,
+//		(control >> 3)&1,
-		(control >> 2)&1,
+//		(control >> 2)&1,
-		(control >> 1)&1,
+//		(control >> 1)&1,
-		(control >> 0)&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;
+	if((changes >> 5)&3)
-	set_drive(drives_[selected_drive_]);
+	{
+		selected_drive_ = (control >> 5)&3;
+		set_drive(drives_[selected_drive_]);
+	}
 
 	// b4: side select
-	unsigned int head = (control & 0x10) ? 1 : 0;
+	if(changes & 0x10)
-	for(int c = 0; c < 4; c++)
 	{
-		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();
+	if(changes & 0x01)
-	irq_enable_ = !!(control & 0x01);
-	bool has_irq = get_interrupt_request_line();
-	if(has_irq != had_irq && delegate_)
 	{
-		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(changes & 0x82)
-	if(new_paging_flags != paging_flags_)
 	{
-		paging_flags_ = new_paging_flags;
+		paging_flags_ = ((control & 0x02) ? 0 : BASICDisable) | ((control & 0x80) ? MicrodscDisable : 0);
 		if(delegate_) delegate_->microdisc_did_change_paging_flags(this);
 	}
 }

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_;
 };
 
 }

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_;

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();
 }
 

Storage/Disk/Encodings/MFM.cpp

@@ -18,6 +18,7 @@ class MFMEncoder: public Encoder {
 		MFMEncoder(std::vector<uint8_t> &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));
+			uint16_t or_bits = (uint16_t)((spread_value << 1) | (spread_value >> 1) | (last_output_ << 15));
-			output_ = spread_value | ((~or_bits) & 0xaaaa);
+			uint16_t output = spread_value | ((~or_bits) & 0xaaaa);
-			output_short(output_);
+			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<uint8_t> &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<class T> std::shared_ptr<Storage::Disk::Track>
 	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<class T> std::shared_ptr<Storage::Disk::Track>
 		shifter.add_byte(sector.sector);
 		uint8_t size = logarithmic_size_for_size(sector.data.size());
 		shifter.add_byte(size);
-
+		shifter.add_crc();
-		// 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);
 
 		// gap
 		for(int c = 0; c < post_address_mark_bytes; c++) shifter.add_byte(0x4e);
@@ -147,17 +148,11 @@ template<class T> std::shared_ptr<Storage::Disk::Track>
 
 		// 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]);
 		}
-
+		shifter.add_crc();
-		// data CRC
-		crc_value = crc_generator.get_value();
-		shifter.add_byte(crc_value >> 8);
-		shifter.add_byte(crc_value & 0xff);
 
 		// gap
 		for(int c = 0; c < post_data_bytes; c++) shifter.add_byte(0x00);
@@ -171,6 +166,7 @@ template<class T> std::shared_ptr<Storage::Disk::Track>
 }
 
 Encoder::Encoder(std::vector<uint8_t> &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::Disk::Track> Storage::Encodings::MFM::GetFMTrackWithSectors(const std::vector<Sector> &sectors)
 {
 	return GetTrackWithSectors<FMEncoder>(
@@ -298,7 +299,7 @@ std::shared_ptr<Storage::Encodings::MFM::Sector> 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<Storage::Encodings::MFM::Sector> 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;

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 MFMIndexSync				= 0x5224;
-const uint16_t MFMAddressMark			= 0x4489;
+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<uint8_t> &target_;