Set product data in device class

uweseimet · Dec 6, 2023 · c22a75a · c22a75a
1 parent 4f71376
commit c22a75a
Show file tree

Hide file tree

Showing 23 changed files with 223 additions and 226 deletions.
diff --git a/.github/workflows/analyze.yml b/.github/workflows/analyze.yml
@@ -5,12 +5,8 @@ on:
     inputs:
       branch:
         required: true
-        type: choice
+        type: string
         description: Branch to analyze
-        options:
-        - develop
-        - main
-        default: develop
 
 jobs:
   code_analysis:
@@ -31,6 +27,7 @@ jobs:
     steps:
       - uses: actions/checkout@v4
         with:
+          ref: $branch
           # Shallow clones should be disabled for a better relevancy of analysis
           fetch-depth: 0
 

diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -1,4 +1,4 @@
-name: Build and test on code changes
+name: Build and test after code change
 
 on:
   workflow_call:

diff --git a/cpp/base/device_factory.cpp b/cpp/base/device_factory.cpp
@@ -6,7 +6,6 @@
 //
 //---------------------------------------------------------------------------
 
-#include "shared/s2p_util.h"
 #if defined BUILD_SCHD || defined BUILD_SCRM
 #include "devices/scsi_hd.h"
 #endif
@@ -49,66 +48,50 @@ shared_ptr<PrimaryDevice> DeviceFactory::CreateDevice(PbDeviceType type, int lun
 #if defined BUILD_SCHD || defined BUILD_SCRM
 	case SCHD: {
 	    const string ext = GetExtensionLowerCase(filename);
-	    device = make_shared<ScsiHd>(lun, false, ext == "hd1" ? scsi_level::scsi_1_ccs : scsi_level::scsi_2);
-
-	    // Some Apple tools require a particular drive identification.
-	    // Except for the vendor string .hda is exactly the same as .hds.
-	    if (ext == "hda") {
-	        device->SetVendor("QUANTUM");
-	        device->SetProduct("FIREBALL");
-		}
+	    device = make_shared<ScsiHd>(lun, false, ext == "hda", ext == "hd1");
 		break;
 	}
 
 	case SCRM:
-		device = make_shared<ScsiHd>(lun, true, scsi_level::scsi_2);
-		device->SetProduct("SCSI HD (REM.)");
+		device = make_shared<ScsiHd>(lun, true, false, false);
 		break;
 #endif
 
 #ifdef BUILD_SCMO
 	case SCMO:
 		device = make_shared<OpticalMemory>(lun);
-		device->SetProduct("SCSI MO");
 		break;
 #endif
 
 #ifdef BUILD_SCCD
-	case SCCD:
-		device = make_shared<ScsiCd>(lun,
-            GetExtensionLowerCase(filename) == "is1" ? scsi_level::scsi_1_ccs : scsi_level::scsi_2);
-		device->SetProduct("SCSI CD-ROM");
+	case SCCD: {
+        const string ext = GetExtensionLowerCase(filename);
+		device = make_shared<ScsiCd>(lun, ext == "is1");
 		break;
+	}
 #endif
 
 #ifdef BUILD_SCDP
 	case SCDP:
 		device = make_shared<SCSIDaynaPort>(lun);
-		// Since this is an emulation for a specific device the full INQUIRY data have to be set accordingly
-		device->SetVendor("Dayna");
-		device->SetProduct("SCSI/Link");
-		device->SetRevision("1.4a");
 		break;
 #endif
 
 #ifdef BUILD_SCHS
 	case SCHS:
 		device = make_shared<HostServices>(lun);
-		device->SetProduct("Host Services");
 		break;
 #endif
 
 #ifdef BUILD_SCLP
 	case SCLP:
 		device = make_shared<Printer>(lun);
-		device->SetProduct("SCSI PRINTER");
 		break;
 #endif
 
 #ifdef BUILD_SAHD
 	case SAHD:
 	    device = make_shared<SasiHd>(lun);
-	    device->SetProduct("SASI HD");
 	    break;
 #endif
 

diff --git a/cpp/devices/daynaport.cpp b/cpp/devices/daynaport.cpp
@@ -30,7 +30,12 @@ using namespace scsi_command_util;
 // The MacOS Daynaport driver needs to have a delay after the size/flags field of the read response
 SCSIDaynaPort::SCSIDaynaPort(int lun) : PrimaryDevice(SCDP, lun, DAYNAPORT_READ_HEADER_SZ)
 {
-	SupportsParams(true);
+    // Since this is an emulation for a specific device the full INQUIRY data have to be set accordingly
+    SetVendor("Dayna");
+    SetProduct("SCSI/Link");
+    SetRevision("1.4a");
+
+    SupportsParams(true);
 }
 
 bool SCSIDaynaPort::Init(const param_map& params)

diff --git a/cpp/devices/host_services.cpp b/cpp/devices/host_services.cpp
@@ -99,6 +99,11 @@ using namespace scsi_defs;
 using namespace scsi_command_util;
 using namespace protobuf_util;
 
+HostServices::HostServices(int lun) : ModePageDevice(SCHS, lun)
+{
+    SetProduct("Host Services");
+}
+
 bool HostServices::Init(const param_map& params)
 {
     ModePageDevice::Init(params);
@@ -108,7 +113,7 @@ bool HostServices::Init(const param_map& params)
     AddCommand(scsi_command::cmd_execute_operation, [this] { ExecuteOperation(); });
     AddCommand(scsi_command::cmd_receive_operation_results, [this] { ReceiveOperationResults(); });
 
-	SetReady(true);
+    SetReady(true);
 
 	return true;
 }

diff --git a/cpp/devices/host_services.h b/cpp/devices/host_services.h
@@ -27,7 +27,7 @@ class HostServices: public ModePageDevice
 
 public:
 
-	explicit HostServices(int lun) : ModePageDevice(SCHS, lun) {}
+	explicit HostServices(int);
 	~HostServices() override = default;
 
 	bool Init(const param_map&) override;

diff --git a/cpp/devices/optical_memory.cpp b/cpp/devices/optical_memory.cpp
@@ -26,10 +26,10 @@ OpticalMemory::OpticalMemory(int lun) : Disk(SCMO, lun, { 512, 1024, 2048, 4096
 	// 640 MB, 20248 bytes per sector, 310352 sectors
 	geometries[2048 * 310352] = { 2048, 310352 };
 
+	SetProduct("SCSI MO");
 	SetProtectable(true);
 	SetRemovable(true);
 	SetLockable(true);
-
 	SupportsSaveParameters(true);
 }
 

diff --git a/cpp/devices/printer.cpp b/cpp/devices/printer.cpp
@@ -40,6 +40,7 @@ using namespace scsi_command_util;
 
 Printer::Printer(int lun) : PrimaryDevice(SCLP, lun)
 {
+    SetProduct("SCSI PRINTER");
 	SupportsParams(true);
 }
 

diff --git a/cpp/devices/sasi_hd.cpp b/cpp/devices/sasi_hd.cpp
@@ -12,6 +12,7 @@
 
 SasiHd::SasiHd(int lun, const unordered_set<uint32_t>& sector_sizes) : Disk(SAHD, lun, sector_sizes)
 {
+    SetProduct("SASI HD");
     SetProtectable(true);
 }
 

diff --git a/cpp/devices/scsi_cd.cpp b/cpp/devices/scsi_cd.cpp
@@ -18,9 +18,11 @@
 using namespace scsi_defs;
 using namespace scsi_command_util;
 
-ScsiCd::ScsiCd(int lun, scsi_defs::scsi_level level) : Disk(SCCD, lun, { 512, 2048 }), scsi_level(level)
+ScsiCd::ScsiCd(int lun, bool scsi1)
+    : Disk(SCCD, lun, { 512, 2048 }), scsi_level(scsi1 ? scsi_level::scsi_1_ccs : scsi_level::scsi_2)
 {
-	SetReadOnly(true);
+    SetProduct("SCSI CD-ROM");
+    SetReadOnly(true);
 	SetRemovable(true);
 	SetLockable(true);
 }

diff --git a/cpp/devices/scsi_cd.h b/cpp/devices/scsi_cd.h
@@ -22,7 +22,7 @@ class ScsiCd : public Disk, private ScsiMmcCommands
 {
 public:
 
-	explicit ScsiCd(int, scsi_defs::scsi_level = scsi_level::scsi_2);
+	explicit ScsiCd(int, bool = false);
 	~ScsiCd() override = default;
 
 	bool Init(const param_map&) override;

diff --git a/cpp/devices/scsi_hd.cpp b/cpp/devices/scsi_hd.cpp
@@ -12,13 +12,20 @@
 
 using namespace scsi_command_util;
 
-ScsiHd::ScsiHd(int lun, bool removable, scsi_defs::scsi_level level, const unordered_set<uint32_t>& sector_sizes)
-	: Disk(removable ? SCRM : SCHD, lun, sector_sizes), scsi_level(level)
+ScsiHd::ScsiHd(int lun, bool removable, bool apple, bool scsi1, const unordered_set<uint32_t>& sector_sizes)
+	: Disk(removable ? SCRM : SCHD, lun, sector_sizes), scsi_level(scsi1 ? scsi_level::scsi_1_ccs : scsi_level::scsi_2)
 {
+    // Some Apple tools require a particular drive identification.
+    // Except for the vendor string .hda is the same as .hds.
+    if (apple) {
+        SetVendor("QUANTUM");
+        SetProduct("FIREBALL");
+    } else if (removable) {
+        SetProduct("SCSI HD (REM.)");
+    }
 	SetProtectable(true);
 	SetRemovable(removable);
 	SetLockable(removable);
-
 	SupportsSaveParameters(true);
 }
 

diff --git a/cpp/devices/scsi_hd.h b/cpp/devices/scsi_hd.h
@@ -21,7 +21,7 @@ class ScsiHd : public Disk
 
 public:
 
-	ScsiHd(int, bool, scsi_defs::scsi_level, const unordered_set<uint32_t>& = { 512, 1024, 2048, 4096 });
+	ScsiHd(int, bool, bool, bool, const unordered_set<uint32_t>& = { 512, 1024, 2048, 4096 });
 	~ScsiHd() override = default;
 
 	void FinalizeSetup(off_t);

diff --git a/cpp/test/daynaport_test.cpp b/cpp/test/daynaport_test.cpp
@@ -10,6 +10,30 @@
 #include "shared/shared_exceptions.h"
 #include "devices/daynaport.h"
 
+TEST(ScsiDaynaportTest, Device_Defaults)
+{
+    DeviceFactory device_factory;
+
+    auto device = device_factory.CreateDevice(UNDEFINED, 0, "daynaport");
+    EXPECT_NE(nullptr, device);
+    EXPECT_EQ(SCDP, device->GetType());
+    EXPECT_FALSE(device->SupportsFile());
+    EXPECT_TRUE(device->SupportsParams());
+    EXPECT_FALSE(device->IsProtectable());
+    EXPECT_FALSE(device->IsProtected());
+    EXPECT_FALSE(device->IsReadOnly());
+    EXPECT_FALSE(device->IsRemovable());
+    EXPECT_FALSE(device->IsRemoved());
+    EXPECT_FALSE(device->IsLockable());
+    EXPECT_FALSE(device->IsLocked());
+    EXPECT_FALSE(device->IsStoppable());
+    EXPECT_FALSE(device->IsStopped());
+
+    EXPECT_EQ("Dayna", device->GetVendor());
+    EXPECT_EQ("SCSI/Link", device->GetProduct());
+    EXPECT_EQ("1.4a", device->GetRevision());
+}
+
 TEST(ScsiDaynaportTest, GetDefaultParams)
 {
 	const auto [controller, daynaport] = CreateDevice(SCDP);
@@ -19,7 +43,7 @@ TEST(ScsiDaynaportTest, GetDefaultParams)
 
 TEST(ScsiDaynaportTest, Inquiry)
 {
-	TestInquiry::Inquiry(SCDP, device_type::processor, scsi_level::scsi_2, "Dayna   SCSI/Link       1.4a", 0x20, false);
+    TestShared::Inquiry(SCDP, device_type::processor, scsi_level::scsi_2, "Dayna   SCSI/Link       1.4a", 0x20, false);
 }
 
 TEST(ScsiDaynaportTest, TestUnitReady)