From bb82dc864a96d66a57ec7a906083e14e32d1b694 Mon Sep 17 00:00:00 2001
From: David Given <dg@cowlark.com>
Date: Sat, 3 Feb 2024 01:08:23 +0100
Subject: [PATCH] Make cross-track sector collation optional (as it usually
 just makes things confusing).

---
 lib/decoders/decoders.cc    |  6 ++++--
 lib/decoders/decoders.proto |  7 ++++---
 lib/readerwriter.cc         | 23 ++++++++++++++++-------
 lib/sector.cc               |  4 ++++
 lib/sector.h                |  1 +
 src/formats/ibm.textpb      |  4 ++++
 6 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/lib/decoders/decoders.cc b/lib/decoders/decoders.cc
index 446f438c..453abbd4 100644
--- a/lib/decoders/decoders.cc
+++ b/lib/decoders/decoders.cc
@@ -137,8 +137,10 @@ std::shared_ptr<TrackDataFlux> Decoder::decodeToSectors(
 
         if (_sector->status != Sector::MISSING)
         {
-            auto trackLayout = Layout::getLayoutOfTrack(
-                _sector->logicalTrack, _sector->logicalSide);
+            if ((_sector->status == Sector::OK) &&
+                ((_sector->logicalTrack != trackInfo->logicalTrack) ||
+                    (_sector->logicalSide != trackInfo->logicalSide)))
+                _sector->status == Sector::WRONG_PLACE;
             _trackdata->sectors.push_back(_sector);
         }
     }
diff --git a/lib/decoders/decoders.proto b/lib/decoders/decoders.proto
index d861b4eb..7d5919bf 100644
--- a/lib/decoders/decoders.proto
+++ b/lib/decoders/decoders.proto
@@ -21,7 +21,7 @@ import "arch/zilogmcz/zilogmcz.proto";
 import "lib/fluxsink/fluxsink.proto";
 import "lib/common.proto";
 
-//NEXT: 32
+//NEXT: 33
 message DecoderProto {
 	optional double pulse_debounce_threshold = 1 [default = 0.30,
 		(help) = "ignore pulses with intervals shorter than this, in fractions of a clock"];
@@ -66,6 +66,7 @@ message DecoderProto {
 	optional string write_csv_to = 23
 		[(help) = "if set, write a CSV report of the disk state"];
 	optional bool skip_unnecessary_tracks = 29 [default = true,
-		(help) = "don't read tracks if we already have all necessary sectors"];
+		(help) = "don't read physical tracks if we already have all necessary sectors"];
+	optional bool collate_sectors_from_all_tracks = 32 [default = false,
+		(help) = "when creating the image, consider all valid sectors even if they're on the wrong track"];
 }
-
diff --git a/lib/readerwriter.cc b/lib/readerwriter.cc
index e1c25ccd..b1ce3a20 100644
--- a/lib/readerwriter.cc
+++ b/lib/readerwriter.cc
@@ -97,9 +97,16 @@ void measureDiskRotation()
     log(EndSpeedOperationLogMessage{oneRevolution});
 }
 
+static int getEffectiveStatus(int status)
+{
+    if ((status == Sector::WRONG_PLACE) &&
+        globalConfig()->decoder().collate_sectors_from_all_tracks())
+        return Sector::OK;
+    return status;
+}
+
 /* Given a set of sectors, deduplicates them sensibly (e.g. if there is a good
  * and bad version of the same sector, the bad version is dropped). */
-
 static std::set<std::shared_ptr<const Sector>> collectSectors(
     std::set<std::shared_ptr<const Sector>>& track_sectors,
     bool collapse_conflicts = true)
@@ -124,9 +131,11 @@ static std::set<std::shared_ptr<const Sector>> collectSectors(
             it->second,
             [&](auto left, auto& rightit) -> std::shared_ptr<const Sector>
             {
+                int leftStatus = left->status;
                 auto& right = rightit.second;
-                if ((left->status == Sector::OK) &&
-                    (right->status == Sector::OK) &&
+                int rightStatus = right->status;
+                if ((leftStatus == Sector::OK) &&
+                    (rightStatus == Sector::OK) &&
                     (left->data != right->data))
                 {
                     if (!collapse_conflicts)
@@ -139,13 +148,13 @@ static std::set<std::shared_ptr<const Sector>> collectSectors(
                     s->status = Sector::CONFLICT;
                     return s;
                 }
-                if (left->status == Sector::CONFLICT)
+                if (leftStatus == Sector::CONFLICT)
                     return left;
-                if (right->status == Sector::CONFLICT)
+                if (rightStatus == Sector::CONFLICT)
                     return right;
-                if (left->status == Sector::OK)
+                if (leftStatus == Sector::OK)
                     return left;
-                if (right->status == Sector::OK)
+                if (rightStatus == Sector::OK)
                     return right;
                 return left;
             });
diff --git a/lib/sector.cc b/lib/sector.cc
index 40f024a7..1c4e5bc3 100644
--- a/lib/sector.cc
+++ b/lib/sector.cc
@@ -31,6 +31,8 @@ std::string Sector::statusToString(Status status)
             return "present but no data found";
         case Status::CONFLICT:
             return "conflicting data";
+        case Status::WRONG_PLACE:
+            return "incorrectly placed";
         default:
             return fmt::format("unknown error {}", status);
     }
@@ -50,6 +52,8 @@ std::string Sector::statusToChar(Status status)
             return "!";
         case Status::CONFLICT:
             return "*";
+        case Status::WRONG_PLACE:
+            return "/";
         default:
             return "?";
     }
diff --git a/lib/sector.h b/lib/sector.h
index a9c34ee2..b0d445eb 100644
--- a/lib/sector.h
+++ b/lib/sector.h
@@ -44,6 +44,7 @@ struct Sector : public LogicalLocation
         DATA_MISSING,
         CONFLICT,
         INTERNAL_ERROR,
+        WRONG_PLACE,
     };
 
     static std::string statusToString(Status status);
diff --git a/src/formats/ibm.textpb b/src/formats/ibm.textpb
index a6a976d6..165ac4ad 100644
--- a/src/formats/ibm.textpb
+++ b/src/formats/ibm.textpb
@@ -109,6 +109,10 @@ option_group {
 		set_by_default: true
 
 		config {
+			decoder {
+				collate_sectors_from_all_tracks: true
+			}
+			
 			layout {
 				format_type: FORMATTYPE_80TRACK
 			}