Inquire the backend for device capabilities instead of Web UI assumptions (#688)

* Add capabilities to RaCtlCmds.get_device_types() to return the image file support boolean, and list of supported block sizes.

* Inquire rascsi backend about the min block size rather than hard coding values in the web UI.

* Add class methods for getting lists of certain device types.

* Use the new class methods to get lists of device types in the web ui.

* Make use of the new class methods in the oled script.

* Remove now unused constants, and simplify logic in common_settings

* Improve device name mapping to extend the existing dictionary rather than creating a new data structure.

* Use jinja2 sort filters instead of sorting in python code. Removing redundant variables.

* Introduce the get_device_name() utility method which returns the translated name for a device acronym. Use the new method to display device name when attaching devices.

* Fix typo

* Rename Support Device to Periperal Device. General tweaks to UI strings.

* Tweak UI string.

* Fix error.

python/common/src/rascsi/common_settings.py

@@ -4,14 +4,8 @@ Module for general settings used in the rascsi module
 
 from os import getcwd
 
-WORK_DIR = getcwd()
-
-REMOVABLE_DEVICE_TYPES = ("SCCD", "SCRM", "SCMO")
-NETWORK_DEVICE_TYPES = ("SCDP", "SCBR")
-SUPPORT_DEVICE_TYPES = ("SCLP", "SCHS")
-
 # There may be a more elegant way to get the HOME dir of the user that installed RaSCSI
-HOME_DIR = "/".join(WORK_DIR.split("/")[0:3])
+HOME_DIR = "/".join(getcwd().split("/")[0:3])
 CFG_DIR = f"{HOME_DIR}/.config/rascsi"
 CONFIG_FILE_SUFFIX = "json"
 

python/common/src/rascsi/ractl_cmds.py

@@ -3,7 +3,6 @@ Module for commands sent to the RaSCSI backend service.
 """
 
 import rascsi_interface_pb2 as proto
-from rascsi.common_settings import REMOVABLE_DEVICE_TYPES
 from rascsi.return_codes import ReturnCodes
 from rascsi.socket_cmds import SocketCmds
 
@@ -137,14 +136,54 @@ class RaCtlCmds:
         result = proto.PbResult()
         result.ParseFromString(data)
         device_types = {}
-        import logging
         for device in result.device_types_info.properties:
             params = {}
             for key, value in device.properties.default_params.items():
                 params[key] = value
-            device_types[proto.PbDeviceType.Name(device.type)] = params
+            device_types[proto.PbDeviceType.Name(device.type)] = {
+                    "removable": device.properties.removable,
+                    "supports_file": device.properties.supports_file,
+                    "params": params,
+                    "block_sizes": device.properties.block_sizes,
+                    }
         return {"status": result.status, "device_types": device_types}
 
+    def get_removable_device_types(self):
+        """
+        Returns a (list) of (str) of four letter device acronyms
+        that are of the removable type.
+        """
+        device_types = self.get_device_types()
+        removable_device_types = []
+        for device, value in device_types["device_types"].items():
+            if value["removable"]:
+                removable_device_types.append(device)
+        return removable_device_types
+
+    def get_disk_device_types(self):
+        """
+        Returns a (list) of (str) of four letter device acronyms 
+        that take image files as arguments.
+        """
+        device_types = self.get_device_types()
+        disk_device_types = []
+        for device, value in device_types["device_types"].items():
+            if value["supports_file"]:
+                disk_device_types.append(device)
+        return disk_device_types
+
+    def get_peripheral_device_types(self):
+        """
+        Returns a (list) of (str) of four letter device acronyms
+        that don't take image files as arguments.
+        """
+        device_types = self.get_device_types()
+        image_device_types = self.get_disk_device_types()
+        peripheral_device_types = [
+                x for x in device_types["device_types"] if x not in image_device_types
+                ]
+        return peripheral_device_types
+
     def get_image_files_info(self):
         """
         Sends a DEFAULT_IMAGE_FILES_INFO command to the server.
@@ -208,7 +247,8 @@ class RaCtlCmds:
         else:
             current_type = None
 
-        if device_type in REMOVABLE_DEVICE_TYPES and current_type in REMOVABLE_DEVICE_TYPES:
+        removable_device_types = self.get_removable_device_types()
+        if device_type in removable_device_types and current_type in removable_device_types:
             if current_type != device_type:
                 parameters = {
                     "device_type": device_type,

python/oled/src/rascsi_oled_monitor.py

@@ -43,12 +43,6 @@ from pi_cmds import get_ip_and_host
 from rascsi.ractl_cmds import RaCtlCmds
 from rascsi.socket_cmds import SocketCmds
 
-from rascsi.common_settings import (
-    REMOVABLE_DEVICE_TYPES,
-    NETWORK_DEVICE_TYPES,
-    SUPPORT_DEVICE_TYPES,
-)
-
 parser = argparse.ArgumentParser(description="RaSCSI OLED Monitor script")
 parser.add_argument(
     "--rotation",
@@ -166,7 +160,8 @@ LINE_SPACING = 8
 FONT = ImageFont.truetype('resources/type_writer.ttf', FONT_SIZE)
 
 IP_ADDR, HOSTNAME = get_ip_and_host()
-
+REMOVABLE_DEVICE_TYPES = ractl_cmd.get_removable_device_types()
+SUPPORT_DEVICE_TYPES = ractl_cmd.get_support_device_types()
 
 def formatted_output():
     """
@@ -188,11 +183,12 @@ def formatted_output():
                 else:
                     output.append(f"{line['id']} {line['device_type'][2:4]} {line['status']}")
             # Special handling of devices that don't use image files
-            elif line["device_type"] in (NETWORK_DEVICE_TYPES):
-                output.append(f"{line['id']} {line['device_type'][2:4]} {line['vendor']} "
-                              f"{line['product']}")
             elif line["device_type"] in (SUPPORT_DEVICE_TYPES):
-                output.append(f"{line['id']} {line['device_type'][2:4]} {line['product']}")
+                if line["vendor"] == "RaSCSI":
+                    output.append(f"{line['id']} {line['device_type'][2:4]} {line['product']}")
+                else:
+                    output.append(f"{line['id']} {line['device_type'][2:4]} {line['vendor']} "
+                                  f"{line['product']}")
             # Print only the Vendor/Product info if it's not generic RaSCSI
             elif line["vendor"] not in "RaSCSI":
                 output.append(f"{line['id']} {line['device_type'][2:4]} {line['file']} "

python/web/src/device_utils.py

@@ -52,33 +52,39 @@ def sort_and_format_devices(devices):
     return formatted_devices
 
 
-def extend_device_names(device_types):
+def map_device_types_and_names(device_types):
     """
-    Takes a (list) of (str) device_types with the four letter device acronyms
+    Takes a (dict) corresponding to the data structure returned by RaCtlCmds.get_device_types()
     Returns a (dict) of device_type:device_name mappings of localized device names
     """
-    mapped_device_types = {}
+    for key, value in device_types.items():
-    for device_type in device_types:
+        device_types[key]["name"] = get_device_name(key)
-        if device_type == "SAHD":
-            device_name = _("SASI Hard Disk")
-        elif device_type == "SCHD":
-            device_name = _("SCSI Hard Disk")
-        elif device_type == "SCRM":
-            device_name = _("Removable Disk")
-        elif device_type == "SCMO":
-            device_name = _("Magneto-Optical")
-        elif device_type == "SCCD":
-            device_name = _("CD / DVD")
-        elif device_type == "SCBR":
-            device_name = _("X68000 Host Bridge")
-        elif device_type == "SCDP":
-            device_name = _("DaynaPORT SCSI/Link")
-        elif device_type == "SCLP":
-            device_name = _("Printer")
-        elif device_type == "SCHS":
-            device_name = _("Host Services")
-        else:
-            device_name = _("Unknown Device")
-        mapped_device_types[device_type] = device_name
 
-    return mapped_device_types
+    return device_types
+
+
+def get_device_name(device_type):
+    """
+    Takes a four letter device acronym (str) device_type.
+    Returns the human-readable name for the device type.
+    """
+    if device_type == "SAHD":
+        return _("SASI Hard Disk")
+    elif device_type == "SCHD":
+        return _("SCSI Hard Disk")
+    elif device_type == "SCRM":
+        return _("Removable Disk")
+    elif device_type == "SCMO":
+        return _("Magneto-Optical")
+    elif device_type == "SCCD":
+        return _("CD / DVD")
+    elif device_type == "SCBR":
+        return _("X68000 Host Bridge")
+    elif device_type == "SCDP":
+        return _("DaynaPORT SCSI/Link")
+    elif device_type == "SCLP":
+        return _("Printer")
+    elif device_type == "SCHS":
+        return _("Host Services")
+    else:
+        return device_type

python/web/src/templates/drives.html

@@ -15,7 +15,7 @@
     <td><b>{{ _("Ref.") }}</b></td>
     <td><b>{{ _("Action") }}</b></td>
 </tr>
-{% for hd in hd_conf %}
+{% for hd in hd_conf|sort(attribute='name') %}
 <tr>
     <td style="text-align:center">{{ hd.name }}</td>
     <td style="text-align:center">{{ hd.size_mb }}</td>
@@ -59,7 +59,7 @@
     <td><b>{{ _("Ref.") }}</b></td>
     <td><b>{{ _("Action") }}</b></td>
 </tr>
-{% for cd in cd_conf %}
+{% for cd in cd_conf|sort(attribute='name') %}
 <tr>
     <td style="text-align:center">{{ cd.name }}</td>
     <td style="text-align:center">{{ cd.size_mb }}</td>
@@ -79,9 +79,9 @@
         <input type="hidden" name="block_size" value="{{ cd.block_size }}">
         <label for="file_name">{{ _("Create for:") }}</label>
     <select type="select" name="file_name">
-        {% for f in files %}
+        {% for file in files|sort(attribute='name') %}
-    {% if f["name"].lower().endswith(cdrom_file_suffix) %}
+    {% if file["name"].lower().endswith(cdrom_file_suffix) %}
-    <option value="{{ f["name"] }}">{{ f["name"].replace(base_dir, '') }}</option>
+    <option value="{{ file["name"] }}">{{ file["name"].replace(base_dir, '') }}</option>
     {% endif %}
     {% endfor %}
     </select>
@@ -105,7 +105,7 @@
     <td><b>{{ _("Ref.") }}</b></td>
     <td><b>{{ _("Action") }}</b></td>
 </tr>
-{% for rm in rm_conf %}
+{% for rm in rm_conf|sort(attribute='name') %}
 <tr>
     <td style="text-align:center">{{ rm.name }}</td>
     <td style="text-align:center">{{ rm.size_mb }}</td>

python/web/src/templates/index.html

@@ -15,7 +15,7 @@
 <p><form action="/config/load" method="post">
     <select name="name" required="" width="14">
         {% if config_files %}
-        {% for config in config_files %}
+        {% for config in config_files|sort %}
         <option value="{{ config }}">
             {{ config.replace(".json", '') }}
         </option>
@@ -155,7 +155,7 @@
         <li>{{ _("Manage image files in the active RaSCSI image directory: <tt>%(directory)s</tt> with a scan depth of %(scan_depth)s.", directory=base_dir, scan_depth=scan_depth) }}</li>
         <li>{{ _("Select a valid SCSI ID and <a href=\"%(url)s\">LUN</a> to attach to. Unless you know what you're doing, always use LUN 0.", url="https://en.wikipedia.org/wiki/Logical_unit_number") }}
         </li>
-        <li>{{ _("If RaSCSI was unable to detect the device type associated with the image, you can choose the type from the dropdown.") }}</li>
+        <li>{{ _("If RaSCSI was unable to detect the media type associated with the image, you get to choose the type from the dropdown.") }}</li>
     </ul>
 </details>
 
@@ -164,9 +164,9 @@
         <tr style="font-weight: bold;">
             <td>{{ _("File") }}</td>
             <td>{{ _("Size") }}</td>
-            <td>{{ _("Actions") }}</td>
+            <td>{{ _("Parameters and Actions") }}</td>
         </tr>
-        {% for file in files %}
+        {% for file in files|sort(attribute='name') %}
         <tr>
             {% if file["prop"] %}
             <td>
@@ -264,12 +264,12 @@
                     {% else %}
                         <select name="type">
                         <option selected disabled value="">
-                        {{ _("Select device type") }}
+                        {{ _("Select media type") }}
                         </option>
                         {% for key, value in device_types.items() %}
-                        {% if key not in (NETWORK_DEVICE_TYPES + SUPPORT_DEVICE_TYPES) %}
+                        {% if key in DISK_DEVICE_TYPES %}
                         <option value="{{ key }}">
-                            {{ value }}
+                            {{ value["name"] }}
                         </option>
                         {% endif %}
                         {% endfor %}
@@ -298,7 +298,7 @@
 <hr/>
 <details>
     <summary class="heading">
-        {{ _("Attach Support Device") }}
+        {{ _("Attach Peripheral Device") }}
     </summary>
     <ul>
         <li>{{ _("<a href=\"%(url1)s\">DaynaPORT SCSI/Link</a> and <a href=\"%(url2)s\">X68000 Host Bridge</a> are network devices.", url1="https://github.com/akuker/RASCSI/wiki/Dayna-Port-SCSI-Link", url2="https://github.com/akuker/RASCSI/wiki/X68000#Host_File_System_driver") }}
@@ -317,21 +317,21 @@
 </details>
 <table border="black" cellpadding="3">
     <tr style="font-weight: bold;">
-        <td>{{ _("Type") }}</td>
+        <td>{{ _("Peripheral") }}</td>
-        <td>{{ _("Actions") }}</td>
+        <td>{{ _("Parameters and Actions") }}</td>
     </tr>
-    {% for type in (NETWORK_DEVICE_TYPES + SUPPORT_DEVICE_TYPES) %}
+    {% for type in PERIPHERAL_DEVICE_TYPES %}
     <tr>
         <td>
-            <div>{{ device_types[type] }}</div>
+            <div>{{ device_types[type]["name"] }}</div>
         </td>
         <td>
             <form action="/scsi/attach_device" method="post">
                 <input name="type" type="hidden" value="{{ type }}">
-                {% for key, value in device_params[type].items() %}
+                {% for key, value in device_types[type]["params"].items() %}
                 <label for="{{ key }}">{{ key }}:</label>
                 {% if value.isnumeric() %}
-                <input name="{{ key }}" type="number" size="{{ value|length }}" placeholder="{{ value }}">
+                <input name="{{ key }}" type="number" size="{{ value|length }}" value="{{ value }}">
                 {% elif key == "interface" %}
                 <select name="interface">
                 {% for if in netinfo["ifs"] %}

python/web/src/web.py

@@ -35,7 +35,8 @@ from pi_cmds import (
 from device_utils import (
     sort_and_format_devices,
     get_valid_scsi_ids,
-    extend_device_names,
+    map_device_types_and_names,
+    get_device_name,
 )
 from return_code_mapper import ReturnCodeMapper
 
@@ -53,9 +54,6 @@ from rascsi.common_settings import (
     CFG_DIR,
     CONFIG_FILE_SUFFIX,
     PROPERTIES_SUFFIX,
-    REMOVABLE_DEVICE_TYPES,
-    NETWORK_DEVICE_TYPES,
-    SUPPORT_DEVICE_TYPES,
     RESERVATIONS,
 )
 from rascsi.ractl_cmds import RaCtlCmds
@@ -109,25 +107,18 @@ def index():
                 ),
             )
 
-    locales = get_supported_locales()
     server_info = ractl.get_server_info()
-    disk = disk_space()
     devices = ractl.list_devices()
-    device_types = ractl.get_device_types()
+    device_types = map_device_types_and_names(ractl.get_device_types()["device_types"])
     image_files = file_cmds.list_images()
     config_files = file_cmds.list_config_files()
 
-    mapped_device_types = extend_device_names(device_types["device_types"].keys())
-
     extended_image_files = []
     for image in image_files["files"]:
         if image["detected_type"] != "UNDEFINED":
-            image["detected_type_name"] = mapped_device_types[image["detected_type"]]
+            image["detected_type_name"] = device_types[image["detected_type"]]["name"]
         extended_image_files.append(image)
 
-    sorted_image_files = sorted(extended_image_files, key=lambda x: x["name"].lower())
-    sorted_config_files = sorted(config_files, key=lambda x: x.lower())
-
     attached_images = []
     units = 0
     # If there are more than 0 logical unit numbers, display in the Web UI
@@ -155,14 +146,14 @@ def index():
 
     return render_template(
         "index.html",
-        locales=locales,
+        locales=get_supported_locales(),
         bridge_configured=is_bridge_setup(),
         netatalk_configured=running_proc("afpd"),
         macproxy_configured=running_proc("macproxy"),
         ip_addr=get_ip_address(),
         devices=formatted_devices,
-        files=sorted_image_files,
+        files=extended_image_files,
-        config_files=sorted_config_files,
+        config_files=config_files,
         base_dir=server_info["image_dir"],
         scan_depth=server_info["scan_depth"],
         CFG_DIR=CFG_DIR,
@@ -179,9 +170,8 @@ def index():
         log_levels=server_info["log_levels"],
         current_log_level=server_info["current_log_level"],
         netinfo=ractl.get_network_info(),
-        device_types=mapped_device_types,
+        device_types=device_types,
-        device_params=device_types["device_types"],
+        free_disk=int(disk_space()["free"] / 1024 / 1024),
-        free_disk=int(disk["free"] / 1024 / 1024),
         valid_file_suffix=valid_file_suffix,
         cdrom_file_suffix=tuple(server_info["sccd"]),
         removable_file_suffix=tuple(server_info["scrm"]),
@@ -190,9 +180,9 @@ def index():
         auth_active=auth_active()["status"],
         ARCHIVE_FILE_SUFFIX=ARCHIVE_FILE_SUFFIX,
         PROPERTIES_SUFFIX=PROPERTIES_SUFFIX,
-        REMOVABLE_DEVICE_TYPES=REMOVABLE_DEVICE_TYPES,
+        REMOVABLE_DEVICE_TYPES=ractl.get_removable_device_types(),
-        NETWORK_DEVICE_TYPES=NETWORK_DEVICE_TYPES,
+        DISK_DEVICE_TYPES=ractl.get_disk_device_types(),
-        SUPPORT_DEVICE_TYPES=SUPPORT_DEVICE_TYPES,
+        PERIPHERAL_DEVICE_TYPES=ractl.get_peripheral_device_types(),
     )
 
 
@@ -201,9 +191,6 @@ def drive_list():
     """
     Sets up the data structures and kicks off the rendering of the drive list page
     """
-    server_info = ractl.get_server_info()
-    disk = disk_space()
-
     # Reads the canonical drive properties into a dict
     # The file resides in the current dir of the web ui process
     drive_properties = Path(DRIVE_PROPERTIES_FILE)
@@ -242,27 +229,23 @@ def drive_list():
             device["size_mb"] = "{:,.2f}".format(device["size"] / 1024 / 1024)
             rm_conf.append(device)
 
-    files = file_cmds.list_images()
-    sorted_image_files = sorted(files["files"], key=lambda x: x["name"].lower())
-    hd_conf = sorted(hd_conf, key=lambda x: x["name"].lower())
-    cd_conf = sorted(cd_conf, key=lambda x: x["name"].lower())
-    rm_conf = sorted(rm_conf, key=lambda x: x["name"].lower())
-
     if "username" in session:
         username = session["username"]
     else:
         username = None
 
+    server_info = ractl.get_server_info()
+
     return render_template(
         "drives.html",
-        files=sorted_image_files,
+        files=file_cmds.list_images()["files"],
         base_dir=server_info["image_dir"],
         hd_conf=hd_conf,
         cd_conf=cd_conf,
         rm_conf=rm_conf,
         running_env=running_env(),
         version=server_info["version"],
-        free_disk=int(disk["free"] / 1024 / 1024),
+        free_disk=int(disk_space()["free"] / 1024 / 1024),
         cdrom_file_suffix=tuple(server_info["sccd"]),
         username=username,
         auth_active=auth_active()["status"],
@@ -500,7 +483,7 @@ def log_level():
 @login_required
 def attach_device():
     """
-    Attaches a support device that doesn't take an image file as argument
+    Attaches a peripheral device that doesn't take an image file as argument
     """
     params = {}
     for item in request.form:
@@ -547,11 +530,8 @@ def attach_device():
     process = ReturnCodeMapper.add_msg(process)
     if process["status"]:
         flash(_(
-            (
+            "Attached %(device_type)s to SCSI ID %(id_number)s LUN %(unit_number)s",
-                "Attached device of type %(device_type)s "
+            device_type=get_device_name(device_type),
-                "to SCSI ID %(id_number)s LUN %(unit_number)s"
-            ),
-            device_type=device_type,
             id_number=scsi_id,
             unit_number=unit,
             ))
@@ -575,15 +555,10 @@ def attach_image():
 
     kwargs = {"unit": int(unit), "params": {"file": file_name}}
 
-    # The most common block size is 512 bytes
+    if device_type:
-    expected_block_size = 512
-
-    if device_type != "":
         kwargs["device_type"] = device_type
-        if device_type == "SCCD":
+        device_types = ractl.get_device_types()
-            expected_block_size = 2048
+        expected_block_size = min(device_types["device_types"][device_type]["block_sizes"])
-        elif device_type == "SAHD":
-            expected_block_size = 256
 
     # Attempt to load the device properties file:
     # same file name with PROPERTIES_SUFFIX appended
@@ -604,8 +579,15 @@ def attach_image():
     process = ractl.attach_device(scsi_id, **kwargs)
     process = ReturnCodeMapper.add_msg(process)
     if process["status"]:
-        flash(_("Attached %(file_name)s to SCSI ID %(id_number)s LUN %(unit_number)s",
+        flash(_((
-            file_name=file_name, id_number=scsi_id, unit_number=unit))
+                "Attached %(file_name)s as %(device_type)s to "
+                "SCSI ID %(id_number)s LUN %(unit_number)s"
+                ),
+            file_name=file_name,
+            device_type=get_device_name(device_type),
+            id_number=scsi_id,
+            unit_number=unit,
+            ))
         if int(file_size) % int(expected_block_size):
             flash(_("The image file size %(file_size)s bytes is not a multiple of "
                   u"%(block_size)s. RaSCSI will ignore the trailing data. "