Disable EPICS attributes and commands with too-long PV names

src/fastcs/backends/epics/ioc.py

@@ -13,6 +13,8 @@
 from fastcs.exceptions import FastCSException
 from fastcs.mapping import Mapping
 
+EPICS_MAX_NAME_LENGTH = 60
+
 
 @dataclass
 class EpicsIOCOptions:
@@ -109,20 +111,48 @@ def _add_sub_controller_pvi_info(pv_prefix: str, parent: BaseController):
 def _create_and_link_attribute_pvs(pv_prefix: str, mapping: Mapping) -> None:
  for single_mapping in mapping.get_controller_mappings():
  path = single_mapping.controller.path
+ invalid_attr_names = []
  for attr_name, attribute in single_mapping.attributes.items():
  pv_name = attr_name.title().replace("_", "")
  _pv_prefix = ":".join([pv_prefix] + path)
+ full_pv_name_length = len(f"{_pv_prefix}:{pv_name}")
 
+ if full_pv_name_length > 60:
+ invalid_attr_names.append(attr_name)
+ continue
  match attribute:
  case AttrRW():
- _create_and_link_read_pv(
- _pv_prefix, f"{pv_name}_RBV", attr_name, attribute
- )
- _create_and_link_write_pv(_pv_prefix, pv_name, attr_name, attribute)
+ if full_pv_name_length >= 57:
+ if full_pv_name_length <= 60:
+ print(
+ f"Not creating PVs for {attr_name} as _RBV PV"
+ f" name would exceed {EPICS_MAX_NAME_LENGTH}"
+ " characters"
+ )
+ invalid_attr_names.append(attr_name)
+ else:
+ _create_and_link_read_pv(
+ _pv_prefix, f"{pv_name}_RBV", attr_name, attribute
+ )
+ _create_and_link_write_pv(
+ _pv_prefix, pv_name, attr_name, attribute
+ )
  case AttrR():
  _create_and_link_read_pv(_pv_prefix, pv_name, attr_name, attribute)
  case AttrW():
  _create_and_link_write_pv(_pv_prefix, pv_name, attr_name, attribute)
+ if invalid_attr_names:
+ print(
+ "Warning: implement logic to shorten PV names in FastCS plugin for"
+ f" controller {single_mapping.controller.path}."
+ )
+ for key in invalid_attr_names:
+ print(
+ f"Not creating PV for {key} as full name would exceed"
+ f" {EPICS_MAX_NAME_LENGTH} characters"
+ )
+ single_mapping.attributes.pop(key)
+ setattr(single_mapping.controller, key, None)
 
 
 def _create_and_link_read_pv(
@@ -192,16 +222,31 @@ def _get_output_record(pv: str, datatype: DataType, on_update: Callable) -> Any:
 def _create_and_link_command_pvs(pv_prefix: str, mapping: Mapping) -> None:
  for single_mapping in mapping.get_controller_mappings():
  path = single_mapping.controller.path
+ invalid_attr_names = []
  for attr_name, method in single_mapping.command_methods.items():
  pv_name = attr_name.title().replace("_", "")
  _pv_prefix = ":".join([pv_prefix] + path)
-
- _create_and_link_command_pv(
- _pv_prefix,
- pv_name,
- attr_name,
- MethodType(method.fn, single_mapping.controller),
+ if len(f"{_pv_prefix}:{pv_name}") > EPICS_MAX_NAME_LENGTH:
+ invalid_attr_names.append(attr_name)
+ else:
+ _create_and_link_command_pv(
+ _pv_prefix,
+ pv_name,
+ attr_name,
+ MethodType(method.fn, single_mapping.controller),
+ )
+ if invalid_attr_names:
+ print(
+ "Warning: implement logic to shorten PV names in FastCS plugin for"
+ f" controller {single_mapping.controller.path}."
  )
+ for key in invalid_attr_names:
+ print(
+ f"Not creating PV for {key} as full name would exceed"
+ f" {EPICS_MAX_NAME_LENGTH} characters"
+ )
+ single_mapping.command_methods.pop(key)
+ setattr(single_mapping.controller, key, None)
 
 
 def _create_and_link_command_pv(

tests/backends/epics/test_ioc.py

@@ -1,13 +1,18 @@
 from pytest_mock import MockerFixture
 
+from fastcs.attributes import AttrR
 from fastcs.backends.epics.ioc import (
  EpicsIOC,
  _add_attr_pvi_info,
  _add_pvi_info,
  _add_sub_controller_pvi_info,
 )
+from fastcs.controller import Controller
+from fastcs.datatypes import Int
 from fastcs.mapping import Mapping
 
+DEVICE = "DEVICE"
+
 
 def test_ioc(mocker: MockerFixture, mapping: Mapping):
  builder = mocker.patch("fastcs.backends.epics.ioc.builder")
@@ -16,37 +21,37 @@ def test_ioc(mocker: MockerFixture, mapping: Mapping):
  "fastcs.backends.epics.ioc._add_sub_controller_pvi_info"
  )
 
- EpicsIOC("DEVICE", mapping)
+ EpicsIOC(DEVICE, mapping)
 
  # Check records are created
- builder.boolIn.assert_called_once_with("DEVICE:ReadBool", ZNAM="OFF", ONAM="ON")
- builder.longIn.assert_any_call("DEVICE:ReadInt")
- builder.aIn.assert_called_once_with("DEVICE:ReadWriteFloat_RBV", PREC=2)
+ builder.boolIn.assert_called_once_with(f"{DEVICE}:ReadBool", ZNAM="OFF", ONAM="ON")
+ builder.longIn.assert_any_call(f"{DEVICE}:ReadInt")
+ builder.aIn.assert_called_once_with(f"{DEVICE}:ReadWriteFloat_RBV", PREC=2)
  builder.aOut.assert_any_call(
- "DEVICE:ReadWriteFloat", always_update=True, on_update=mocker.ANY, PREC=2
+ f"{DEVICE}:ReadWriteFloat", always_update=True, on_update=mocker.ANY, PREC=2
  )
- builder.longIn.assert_any_call("DEVICE:ReadWriteInt_RBV")
+ builder.longIn.assert_any_call(f"{DEVICE}:ReadWriteInt_RBV")
  builder.longOut.assert_called_with(
- "DEVICE:ReadWriteInt", always_update=True, on_update=mocker.ANY
+ f"{DEVICE}:ReadWriteInt", always_update=True, on_update=mocker.ANY
  )
- builder.longStringIn.assert_called_once_with("DEVICE:StringEnum_RBV")
+ builder.longStringIn.assert_called_once_with(f"{DEVICE}:StringEnum_RBV")
  builder.longStringOut.assert_called_once_with(
- "DEVICE:StringEnum", always_update=True, on_update=mocker.ANY
+ f"{DEVICE}:StringEnum", always_update=True, on_update=mocker.ANY
  )
  builder.boolOut.assert_called_once_with(
- "DEVICE:WriteBool",
+ f"{DEVICE}:WriteBool",
  ZNAM="OFF",
  ONAM="ON",
  always_update=True,
  on_update=mocker.ANY,
  )
  builder.aOut.assert_any_call(
- "DEVICE:Go", initial_value=0, always_update=True, on_update=mocker.ANY
+ f"{DEVICE}:Go", initial_value=0, always_update=True, on_update=mocker.ANY
  )
 
  # Check info tags are added
- add_pvi_info.assert_called_once_with("DEVICE:PVI")
- add_sub_controller_pvi_info.assert_called_once_with("DEVICE", mapping.controller)
+ add_pvi_info.assert_called_once_with(f"{DEVICE}:PVI")
+ add_sub_controller_pvi_info.assert_called_once_with(DEVICE, mapping.controller)
 
 
 def test_add_pvi_info(mocker: MockerFixture):
@@ -57,18 +62,18 @@ def test_add_pvi_info(mocker: MockerFixture):
  child.path = ["Child"]
  controller.get_sub_controllers.return_value = {"d": child}
 
- _add_pvi_info("DEVICE:PVI")
+ _add_pvi_info(f"{DEVICE}:PVI")
 
  builder.longStringIn.assert_called_once_with(
- "DEVICE:PVI_PV",
- initial_value="DEVICE:PVI",
+ f"{DEVICE}:PVI_PV",
+ initial_value=f"{DEVICE}:PVI",
  DESC="The records in this controller",
  )
  record = builder.longStringIn.return_value
  record.add_info.assert_called_once_with(
  "Q:group",
  {
- "DEVICE:PVI": {
+ f"{DEVICE}:PVI": {
  "+id": "epics:nt/NTPVI:1.0",
  "display.description": {"+type": "plain", "+channel": "DESC"},
  "": {"+type": "meta", "+channel": "VAL"},
@@ -86,23 +91,23 @@ def test_add_pvi_info_with_parent(mocker: MockerFixture):
  controller.get_sub_controllers.return_value = {"d": child}
 
  child = mocker.MagicMock()
- _add_pvi_info("DEVICE:Child:PVI", "DEVICE:PVI", "child")
+ _add_pvi_info(f"{DEVICE}:Child:PVI", f"{DEVICE}:PVI", "child")
 
  builder.longStringIn.assert_called_once_with(
- "DEVICE:Child:PVI_PV",
- initial_value="DEVICE:Child:PVI",
+ f"{DEVICE}:Child:PVI_PV",
+ initial_value=f"{DEVICE}:Child:PVI",
  DESC="The records in this controller",
  )
  record = builder.longStringIn.return_value
  record.add_info.assert_called_once_with(
  "Q:group",
  {
- "DEVICE:Child:PVI": {
+ f"{DEVICE}:Child:PVI": {
  "+id": "epics:nt/NTPVI:1.0",
  "display.description": {"+type": "plain", "+channel": "DESC"},
  "": {"+type": "meta", "+channel": "VAL"},
  },
- "DEVICE:PVI": {
+ f"{DEVICE}:PVI": {
  "value.child.d": {
  "+channel": "VAL",
  "+type": "plain",
@@ -121,20 +126,22 @@ def test_add_sub_controller_pvi_info(mocker: MockerFixture):
  child.path = ["Child"]
  controller.get_sub_controllers.return_value = {"d": child}
 
- _add_sub_controller_pvi_info("DEVICE", controller)
+ _add_sub_controller_pvi_info(DEVICE, controller)
 
- add_pvi_info.assert_called_once_with("DEVICE:Child:PVI", "DEVICE:PVI", "child")
+ add_pvi_info.assert_called_once_with(
+ f"{DEVICE}:Child:PVI", f"{DEVICE}:PVI", "child"
+ )
 
 
 def test_add_attr_pvi_info(mocker: MockerFixture):
  record = mocker.MagicMock()
 
- _add_attr_pvi_info(record, "DEVICE", "attr", "r")
+ _add_attr_pvi_info(record, DEVICE, "attr", "r")
 
  record.add_info.assert_called_once_with(
  "Q:group",
  {
- "DEVICE:PVI": {
+ f"{DEVICE}:PVI": {
  "value.attr.r": {
  "+channel": "NAME",
  "+type": "plain",
@@ -143,3 +150,39 @@ def test_add_attr_pvi_info(mocker: MockerFixture):
  }
  },
  )
+
+
+class ControllerLongNames(Controller):
+ attr_r_with_reallyreallyreallyreallyreallyreallyreally_long_name = AttrR(Int())
+ attr_r_short_name = AttrR(Int())
+
+
+def test_long_pv_names_discarded(mocker: MockerFixture):
+ mocker.patch("fastcs.backends.epics.ioc.builder")
+ long_name_controller = ControllerLongNames()
+ long_name_mapping = Mapping(long_name_controller)
+ assert (
+ "attr_r_with_reallyreallyreallyreallyreallyreallyreally_long_name"
+ in long_name_mapping.get_controller_mappings()[0].attributes
+ )
+ assert hasattr(
+ long_name_mapping.controller,
+ "attr_r_with_reallyreallyreallyreallyreallyreallyreally_long_name",
+ )
+ assert (
+ "attr_r_short_name" in long_name_mapping.get_controller_mappings()[0].attributes
+ )
+ assert hasattr(long_name_mapping.controller, "attr_r_short_name")
+ EpicsIOC(DEVICE, long_name_mapping)
+ assert (
+ "attr_r_with_reallyreallyreallyreallyreallyreallyreally_long_name"
+ not in long_name_mapping.get_controller_mappings()[0].attributes
+ )
+ assert (
+ getattr(
+ long_name_mapping.controller,
+ "attr_r_with_reallyreallyreallyreallyreallyreallyreally_long_name",
+ None,
+ )
+ is None
+ )