Merge pull request #313 from davidgiven/ibm

swap_sides isn't symmetrical for the encoder and the decoder

arch/ibm/decoder.cc

@@ -148,7 +148,7 @@ public:
 			_sector->status = Sector::DATA_MISSING; /* correct but unintuitive */
 
 		if (_config.swap_sides())
-			_sector->logicalSide = 1 - _sector->logicalSide;
+			_sector->logicalSide ^= 1;
 		if (_config.ignore_side_byte())
 			_sector->logicalSide = _sector->physicalHead;
 	}

arch/ibm/encoder.cc

@@ -115,10 +115,11 @@ public:
 		IbmEncoderProto::TrackdataProto trackdata;
 		getTrackFormat(trackdata, physicalTrack, physicalSide);
 
+		int logicalSide = physicalSide ^ trackdata.swap_sides();
 		for (char sectorChar : trackdata.sector_skew())
         {
 			int sectorId = charToInt(sectorChar);
-			const auto& sector = image.get(physicalTrack, physicalSide, sectorId);
+			const auto& sector = image.get(physicalTrack, logicalSide, sectorId);
 			if (sector)
 				sectors.push_back(sector);
         }
@@ -147,8 +148,6 @@ public:
 				writeBytes(bytes);
 		};
 
-		if (trackdata.swap_sides())
-			physicalSide = 1 - physicalSide;
 		double clockRateUs = 1e3 / trackdata.clock_rate_khz();
 		if (!trackdata.use_fm())
 			clockRateUs /= 2.0;
@@ -184,6 +183,7 @@ public:
 			writeFillerBytes(trackdata.gap1(), gapFill);
 		}
 
+		int logicalSide = physicalSide ^ trackdata.swap_sides();
 		bool first = true;
 		for (char sectorChar : trackdata.sector_skew())
 		{
@@ -192,7 +192,7 @@ public:
 				writeFillerBytes(trackdata.gap3(), gapFill);
 			first = false;
 
-			const auto& sectorData = image.get(physicalTrack, physicalSide, sectorId);
+			const auto& sectorData = image.get(physicalTrack, logicalSide, sectorId);
 			if (!sectorData)
 			{
 				/* If there are any missing sectors, this is an empty track. */

arch/ibm/ibm.proto

@@ -6,7 +6,7 @@ message IbmDecoderProto {
 	optional int32 sector_id_base = 1        [default = 1, (help) = "ID of first sector"];
 	optional bool ignore_side_byte = 2       [default = false, (help) = "ignore side byte in sector header"];
 	optional RangeProto required_sectors = 3 [(help) = "require these sectors to exist for a good read"];
-	optional bool swap_sides = 4             [default = false, (help) = "swap side bytes when reading"];
+	optional bool swap_sides = 4             [default = false, (help) = "put logical side 1 on physical side 0"];
 }
 
 message IbmEncoderProto {

lib/decoders/decoders.cc

@@ -59,12 +59,12 @@ std::unique_ptr<AbstractDecoder> AbstractDecoder::create(const DecoderProto& con
 }
 
 std::unique_ptr<TrackDataFlux> AbstractDecoder::decodeToSectors(
-		std::shared_ptr<const Fluxmap> fluxmap, unsigned cylinder, unsigned head)
+		std::shared_ptr<const Fluxmap> fluxmap, unsigned physicalCylinder, unsigned physicalHead)
 {
 	_trackdata = std::make_unique<TrackDataFlux>();
 	_trackdata->fluxmap = fluxmap;
-	_trackdata->physicalCylinder = cylinder;
-	_trackdata->physicalHead = head;
+	_trackdata->physicalCylinder = physicalCylinder;
+	_trackdata->physicalHead = physicalHead;
 	
     FluxmapReader fmr(*fluxmap);
     _fmr = &fmr;
@@ -74,8 +74,8 @@ std::unique_ptr<TrackDataFlux> AbstractDecoder::decodeToSectors(
     {
 		_sector = std::make_shared<Sector>();
 		_sector->status = Sector::MISSING;
-		_sector->physicalCylinder = cylinder;
-		_sector->physicalHead = head;
+		_sector->physicalCylinder = physicalCylinder;
+		_sector->physicalHead = physicalHead;
 
         Fluxmap::Position recordStart = fmr.tell();
         RecordType r = advanceToNextRecord();

src/formats/atarist360.textpb

@@ -34,7 +34,7 @@ encoder {
 
 decoder {
 	ibm {
-		swap_sides: true
+		swap_sides: false
 	}
 }
 

src/formats/atarist370.textpb

@@ -34,7 +34,7 @@ encoder {
 
 decoder {
 	ibm {
-		swap_sides: true
+		swap_sides: false
 	}
 }
 

src/formats/atarist400.textpb

@@ -34,7 +34,7 @@ encoder {
 
 decoder {
 	ibm {
-		swap_sides: true
+		swap_sides: false
 	}
 }
 

src/formats/atarist410.textpb

@@ -34,7 +34,7 @@ encoder {
 
 decoder {
 	ibm {
-		swap_sides: true
+		swap_sides: false
 	}
 }
 

src/formats/atarist720.textpb

@@ -34,7 +34,7 @@ encoder {
 
 decoder {
 	ibm {
-		swap_sides: true
+		swap_sides: false
 	}
 }
 

src/formats/atarist740.textpb

@@ -34,7 +34,7 @@ encoder {
 
 decoder {
 	ibm {
-		swap_sides: true
+		swap_sides: false
 	}
 }
 

src/formats/atarist800.textpb

@@ -34,7 +34,7 @@ encoder {
 
 decoder {
 	ibm {
-		swap_sides: true
+		swap_sides: false
 	}
 }
 

src/formats/atarist820.textpb

@@ -34,7 +34,7 @@ encoder {
 
 decoder {
 	ibm {
-		swap_sides: true
+		swap_sides: false
 	}
 }
 

src/formats/commodore1581.textpb

@@ -34,12 +34,15 @@ encoder {
 			gap2: 22
 			gap3: 34
 			sector_skew: "0123456789"
+			swap_sides: true
 		}
 	}
 }
 
 decoder {
-	ibm {}
+	ibm {
+		swap_sides: false
+	}
 }
 
 cylinders {