Merge pull request #104 from epics-containers/ibek-dev

Improvements made while integrating latest pvi with ibek

src/pvi/__main__.py

@@ -10,7 +10,7 @@
 from pvi._convert.utils import extract_device_and_parent_class
 from pvi._format import Formatter
 from pvi._format.template import format_template
-from pvi._pv_group import group_parameters
+from pvi._pv_group import group_by_ui
 from pvi.device import Device
 
 app = typer.Typer()
@@ -117,7 +117,7 @@ def regroup(
 ):
     """Regroup a device.yaml file based on ui files that the PVs appear in"""
     device = Device.deserialize(device_path)
-    device.children = group_parameters(device, ui_paths)
+    device.children = group_by_ui(device, ui_paths)
 
     device.serialize(device_path)
 

src/pvi/_convert/_asyn_convert.py

@@ -29,7 +29,7 @@ def model_post_init(self, __context: Any):
         if all(k in self.fields.keys() for k in ("INP", "OUT")) or not any(
             k in self.fields.keys() for k in ("INP", "OUT")
         ):
-            raise RecordError("Record has no input or output field or both")
+            raise RecordError(f"Record {self.pv} has no input or output field or both")
 
         asyn_field = self.fields.get("INP", self.fields.get("OUT"))
         if asyn_field is None or "@asyn(" not in asyn_field:

src/pvi/_format/base.py

@@ -84,6 +84,7 @@ def format_index(self, label: str, index_entries: list[IndexEntry], path: Path):
                 children=[
                     DeviceRef(
                         name=enforce_pascal_case(index.label),
+                        label=index.label,
                         pv=index.label.upper(),
                         ui=index.ui,
                         macros=index.macros,

src/pvi/_format/screen.py

@@ -529,7 +529,7 @@ def generate_row_component_formatters(
             elif isinstance(rc, DeviceRef):
                 yield self.widget_formatter_factory.sub_screen_formatter_cls(
                     bounds=rc_bounds,
-                    label=rc.get_label(),
+                    label=" ".join(rc.macros.values()),
                     file_name=rc.ui,
                     macros=rc.macros,
                 )

src/pvi/_pv_group.py

@@ -2,7 +2,15 @@
 from pathlib import Path
 from typing import Dict, List, Tuple
 
-from pvi.device import Component, ComponentUnion, Device, Grid, Group, walk
+from pvi.device import (
+    ComponentUnion,
+    Device,
+    Grid,
+    Group,
+    Tree,
+    enforce_pascal_case,
+    walk,
+)
 
 
 def find_pvs(pvs: List[str], file_path: Path) -> Tuple[List[str], List[str]]:
@@ -51,59 +59,48 @@ def find_pvs(pvs: List[str], file_path: Path) -> Tuple[List[str], List[str]]:
     return grouped_pvs, remaining_pvs
 
 
-def sanitize_name(name: str) -> str:
-    name = name.replace(" ", "")
-    name = name.replace("-", "")
-    name = name.replace("_", "")
-    return name
+def group_by_ui(device: Device, ui_paths: List[Path]) -> Tree:
+    signals: List[ComponentUnion] = list(walk(device.children))
 
-
-def group_parameters(device: Device, ui_paths: List[Path]) -> List[Group]:
-    initial_parameters: List[ComponentUnion] = [
-        param for param in walk(device.children) if isinstance(param, Component)
-    ]
-
-    # Group PVs that appear in the given ui files
-    pvs = [node.name for node in walk(device.children) if isinstance(node, Component)]
+    # PVs without macros to search for in UI
+    pv_names = [s.name for s in signals]
 
     group_pv_map: Dict[str, List[str]] = {}
     for ui in ui_paths:
-        ui_pvs, pvs = find_pvs(pvs, ui)
+        ui_pvs, pv_names = find_pvs(pv_names, ui)
         if ui_pvs:
             group_pv_map[ui.stem] = ui_pvs
 
-    if pvs:
-        print(f'Did not find group for {"|".join(pvs)}')
+    if pv_names:
+        print(f"Did not find group for {' | '.join(pv_names)}")
 
     # Create groups for parameters we found in the files
     ui_groups: List[Group] = [
         Group(
-            name=sanitize_name(group_name),
+            name=enforce_pascal_case(group_name),
             layout=Grid(labelled=True),
             children=[  # Note: Need to preserve order in group_pvs here
-                param
-                for pv in group_pvs
-                for param in initial_parameters
-                if param.name == pv
+                signal
+                for pv_name in group_pvs
+                for signal in signals
+                if signal.name == pv_name
             ],
-            label=group_name,
+            label=group_name if enforce_pascal_case(group_name) != group_name else None,
         )
         for group_name, group_pvs in group_pv_map.items()
     ]
 
     # Separate any parameters we failed to find a group for
-    grouped_parameters = [param for group in ui_groups for param in group.children]
-    ungrouped_parameters = [
-        param for param in initial_parameters if param not in grouped_parameters
-    ]
+    grouped_pvs = [pv_name for group in ui_groups for pv_name in group.children]
+    ungrouped_pvs = [pv_name for pv_name in signals if pv_name not in grouped_pvs]
 
-    # Replace with grouped parameters and any ungrouped parameters on the end
-    if ungrouped_parameters:
+    # Add any ungrouped parameters on the end
+    if ungrouped_pvs:
         ui_groups.append(
             Group(
-                name=sanitize_name(device.label + "Misc"),
+                name=enforce_pascal_case(device.label) + "Misc",
                 layout=Grid(labelled=True),
-                children=ungrouped_parameters,
+                children=ungrouped_pvs,
                 label=device.label + " Ungrouped",
             )
         )

src/pvi/device.py

@@ -55,12 +55,21 @@ def to_snake_case(pascal_s: str) -> str:
 def enforce_pascal_case(s: str) -> str:
     """Enforce a pascal case string, removing any invalid characters.
 
+    String will be returned unchanged if it is already valid PascalCase.
+
     Args:
         s: String to convert
 
     Returns: PascalCase string
 
     """
+    if PASCAL_CASE_REGEX.fullmatch(s) is not None:
+        # Already valid
+        return s
+
+    # Do minimal conversion to PascalCase:
+    # - Remove any invalid characters
+    # - Make first character upper case
     s = NON_PASCAL_CHARS_RE.sub(lambda _: "", s)
     return s[0].upper() + s[1:]
 
@@ -402,9 +411,10 @@ class Group(Component):
 def walk(tree: Tree) -> Iterator[ComponentUnion]:
     """Depth first traversal of tree"""
     for t in tree:
-        yield t
         if isinstance(t, Group):
             yield from walk(t.children)
+        else:
+            yield t
 
 
 class Device(TypedModel, YamlValidatorMixin):

tests/format/output/device_ref.bob

@@ -45,7 +45,7 @@
         </macros>
       </action>
     </actions>
-    <text>Combo Box</text>
+    <text>TEST:</text>
     <x>146</x>
     <y>30</y>
     <width>124</width>

tests/format/output/index.bob

@@ -27,7 +27,7 @@
   </widget>
   <widget type="label" version="2.0.0">
     <name>Label</name>
-    <text>Button</text>
+    <text>Button - $(P)</text>
     <x>23</x>
     <y>30</y>
     <width>150</width>
@@ -45,7 +45,7 @@
         </macros>
       </action>
     </actions>
-    <text>Button</text>
+    <text>TEST:</text>
     <x>178</x>
     <y>30</y>
     <width>205</width>
@@ -54,7 +54,7 @@
   </widget>
   <widget type="label" version="2.0.0">
     <name>Label</name>
-    <text>Combo Box</text>
+    <text>ComboBox</text>
     <x>23</x>
     <y>55</y>
     <width>150</width>
@@ -72,7 +72,7 @@
         </macros>
       </action>
     </actions>
-    <text>Combo Box</text>
+    <text>TEST:</text>
     <x>178</x>
     <y>55</y>
     <width>205</width>
@@ -81,7 +81,7 @@
   </widget>
   <widget type="label" version="2.0.0">
     <name>Label</name>
-    <text>Table</text>
+    <text>table</text>
     <x>23</x>
     <y>80</y>
     <width>150</width>
@@ -99,7 +99,7 @@
         </macros>
       </action>
     </actions>
-    <text>Table</text>
+    <text>TEST:</text>
     <x>178</x>
     <y>80</y>
     <width>205</width>

tests/test_api.py

@@ -255,7 +255,7 @@ def test_index(tmp_path, helper):
     DLSFormatter().format_index(
         "Index",
         [
-            IndexEntry(label="Button", ui="button.bob", macros={"P": "TEST:"}),
+            IndexEntry(label="Button - $(P)", ui="button.bob", macros={"P": "TEST:"}),
             IndexEntry(label="ComboBox", ui="combo_box.bob", macros={"P": "TEST:"}),
             # Check that lower case name is OK and will be capitalized to avoid
             # PascalCase validation error