Skip to content

Commit

Permalink
Fix import error on mac with system integrity protection (tektronix#109)
Browse files Browse the repository at this point in the history
* fix: Update helper functions to account for System Integrity Protection on macOS which can cause errors when tm_devices is imported.

* ci: Update pre-commit versions.

* test: Update test to properly run tests from all three supported operating systems.

Signed-off-by: qthompso <[email protected]>
  • Loading branch information
nfelt14 authored and qthompso committed Feb 15, 2024
1 parent 819b012 commit ce9d931
Show file tree
Hide file tree
Showing 8 changed files with 87 additions and 39 deletions.
6 changes: 3 additions & 3 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ repos:
- id: remove-tabs
- id: forbid-tabs
- repo: https://github.com/python-jsonschema/check-jsonschema
rev: 0.27.1
rev: 0.27.2
hooks:
- id: check-readthedocs
- id: check-dependabot
Expand All @@ -50,7 +50,7 @@ repos:
hooks:
- id: blacken-docs
- repo: https://github.com/lyz-code/yamlfix/
rev: 1.15.0
rev: 1.16.0
hooks:
- id: yamlfix
- repo: https://github.com/executablebooks/mdformat
Expand Down Expand Up @@ -122,7 +122,7 @@ repos:
always_run: true
args: [., --min=10]
- repo: https://github.com/charliermarsh/ruff-pre-commit
rev: v0.1.4
rev: v0.1.6
hooks:
- id: ruff
args: [--fix, --exit-non-zero-on-fix]
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ Things to be included in the next release go here.

- Updated the package classifiers for PyPI

### Fixed

- Fixed a crash observed on macOS when importing `tm_devices`, issue [#108](https://github.com/tektronix/tm_devices/issues/108)

______________________________________________________________________

## v1.0.0 (2023-11-13)
Expand Down
2 changes: 1 addition & 1 deletion src/tm_devices/commands/_helpers/generic_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def __missing__(self, key: Any) -> Any:
# noinspection PyArgumentList # pylint: disable=not-callable
dict.__setitem__(self, key, self.default_factory(key)) # type: ignore
return cast(Any, self[key])
return cast(Any, super().__missing__(key)) # pyright: ignore [reportUnknownMemberType]
return cast(Any, super().__missing__(key))


@total_ordering # If comparisons are slowing down the code, implementing the rest would speed it up
Expand Down
2 changes: 1 addition & 1 deletion src/tm_devices/components/dm_config_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ def __parse_env_devices(self) -> List[Dict[str, Any]]:
retval: List[Dict[str, Any]] = []
msg: List[str] = []
for dev_entry in devices_str_list:
temp_dict = {}
temp_dict: Dict[str, Any] = {}
for dev_arg in dev_entry.split(","):
try:
dev_key, dev_val = dev_arg.split("=", 1)
Expand Down
21 changes: 6 additions & 15 deletions src/tm_devices/drivers/pi/pi_device.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
from tm_devices.helpers.constants_and_dataclasses import UNIT_TEST_TIMEOUT


# pylint: disable=too-many-instance-attributes,too-many-public-methods
# pylint: disable=too-many-public-methods
class PIDevice(Device, ABC):
"""Base Programmable Interface (PI) device driver."""

Expand Down Expand Up @@ -64,7 +64,6 @@ def __init__(
elif self._visa_library_path.endswith(".yaml"): # pragma: no cover
# Mark this as a simulated VISA backend
self._visa_library_path += "@sim"
self._visa_backend = self._get_visa_backend()
# Use a default timeout of 30 seconds, if running unit tests use a smaller amount.
self._default_visa_timeout = (
30000
Expand Down Expand Up @@ -159,11 +158,6 @@ def resource_expression(self) -> str:
"""Return the VISA resource expression."""
return self._resource_expression

@property
def visa_backend(self) -> str:
"""Return the VISA backend in use."""
return self._visa_backend

@property
def visa_timeout(self) -> float:
"""Return the current VISA timeout of the device in milliseconds."""
Expand Down Expand Up @@ -237,6 +231,11 @@ def series(self) -> str:
"""
return get_model_series(self.model)

@cached_property
def visa_backend(self) -> str:
"""Return the VISA backend in use."""
return get_visa_backend(self._visa_resource.visalib.library_path.path)

################################################################################################
# Context Manager Methods
################################################################################################
Expand Down Expand Up @@ -819,14 +818,6 @@ def _close(self) -> None:
self._visa_resource = None # type: ignore
self._is_open = False

def _get_visa_backend(self) -> str:
"""Determine what the VISA backend is for this device.
Returns:
A string containing the VISA backend.
"""
return get_visa_backend(self._visa_resource.visalib.library_path.path)

def _has_errors(self) -> bool:
"""Check if the device has any errors.
Expand Down
4 changes: 2 additions & 2 deletions src/tm_devices/drivers/pi/scopes/tekscope/tekscope.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from dataclasses import dataclass
from functools import cached_property
from types import MappingProxyType
from typing import Any, cast, List, Literal, Optional, Tuple, Type, Union
from typing import Any, cast, Dict, List, Literal, Optional, Tuple, Type, Union

import pyvisa as visa

Expand Down Expand Up @@ -125,7 +125,7 @@ def __init__(
def channel(self) -> "MappingProxyType[str, TekScopeChannel]":
"""Mapping of channel names to any detectable properties, attributes, and settings."""
# TODO: overwrite in MSO2 driver, would remove need for try-except
channel_map = {}
channel_map: Dict[str, TekScopeChannel] = {}

with self.temporary_verbose(False) and self.temporary_visa_timeout(
500 if not bool(os.environ.get("TM_DEVICES_UNIT_TESTS_RUNNING")) else UNIT_TEST_TIMEOUT
Expand Down
26 changes: 24 additions & 2 deletions src/tm_devices/helpers/functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import warnings

from enum import EnumMeta
from functools import lru_cache
from typing import Any, Dict, Optional, Tuple, Type

import requests
Expand Down Expand Up @@ -42,7 +43,6 @@
####################################################################################################
# Private Constants
####################################################################################################
_VISA_SYSTEM_DETAILS: Dict[str, Any] = pyvisa_util.get_system_details()
_KEITHLEY_2_CHAR_MODEL_LOOKUP = {
"24": "SMU",
"26": "SMU",
Expand Down Expand Up @@ -436,7 +436,7 @@ def get_visa_backend(visa_lib_path: str) -> str:
"""
visa_name = ""

system_visa_info = _VISA_SYSTEM_DETAILS
system_visa_info = _get_system_visa_info()
# noinspection PyTypeChecker
visa_backends: Dict[str, Any] = system_visa_info["backends"]

Expand Down Expand Up @@ -603,3 +603,25 @@ def _configure_visa_object(
setattr(serial_config, name, getattr(visa_object, name))

return visa_object


@lru_cache(maxsize=None)
def _get_system_visa_info() -> Dict[str, Any]:
"""Get the VISA information for the current system.
Returns:
A dictionary with the VISA info for the system.
"""
fetch_backend_info = True

if platform.system().lower() == "darwin":
try:
output = subprocess.check_output(shlex.split("csrutil status")).decode( # noqa: S603
"utf-8"
)
except subprocess.SubprocessError:
output = ""
if "System Integrity Protection status: enabled" in output:
fetch_backend_info = False

return pyvisa_util.get_system_details(backends=fetch_backend_info)
61 changes: 46 additions & 15 deletions tests/test_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@

from collections import OrderedDict
from contextlib import redirect_stdout
from copy import deepcopy
from io import StringIO
from subprocess import CalledProcessError
from subprocess import CalledProcessError, SubprocessError
from typing import Any, Dict, Optional, Tuple
from unittest import mock

Expand Down Expand Up @@ -309,9 +308,9 @@ def test_check_for_update(capsys: pytest.CaptureFixture[str]) -> None:

def test_get_visa_backend() -> None:
"""Verify that the VISA backend can be determined properly."""
import tm_devices.helpers.functions # pylint: disable=import-outside-toplevel

old_system_details = deepcopy(tm_devices.helpers.functions._VISA_SYSTEM_DETAILS) # noqa: SLF001
from tm_devices.helpers.functions import ( # pylint: disable=import-outside-toplevel
_get_system_visa_info,
)

testing_system_details: Dict[Any, Any] = {
"pyvisa": "1.12.0",
Expand Down Expand Up @@ -363,18 +362,50 @@ def test_get_visa_backend() -> None:
),
}

# Modify the constant for testing purposes
tm_devices.helpers.functions._VISA_SYSTEM_DETAILS = testing_system_details # noqa: SLF001

try:
assert get_visa_backend("tests/sim_devices/devices.yaml") == "PyVISA-sim"
assert get_visa_backend("py") == "PyVISA-py"
assert get_visa_backend("C:\\WINDOWS\\system32\\visa32.dll") == "NI-VISA"
assert get_visa_backend("C:\\WINDOWS\\system32\\visa.dll") == "Custom Vendor VISA"
assert get_visa_backend("nothing") == ""
_get_system_visa_info.cache_clear()
with mock.patch(
"pyvisa.util.get_system_details",
mock.MagicMock(return_value=testing_system_details),
), mock.patch(
"platform.system",
mock.MagicMock(return_value="windows"),
):
assert get_visa_backend("tests/sim_devices/devices.yaml") == "PyVISA-sim"
assert get_visa_backend("py") == "PyVISA-py"
assert get_visa_backend("C:\\WINDOWS\\system32\\visa32.dll") == "NI-VISA"
assert get_visa_backend("C:\\WINDOWS\\system32\\visa.dll") == "Custom Vendor VISA"
assert get_visa_backend("nothing") == ""

with mock.patch(
"platform.system",
mock.MagicMock(return_value="darwin"),
):
_get_system_visa_info.cache_clear()
with mock.patch(
"subprocess.check_output",
mock.MagicMock(return_value=b"System Integrity Protection status: enabled."),
):
assert get_visa_backend("tests/sim_devices/devices.yaml") == ""
assert get_visa_backend("py") == "PyVISA-py"

_get_system_visa_info.cache_clear()
with mock.patch(
"subprocess.check_output",
mock.MagicMock(return_value=b"System Integrity Protection status: disabled."),
):
assert get_visa_backend("tests/sim_devices/devices.yaml") == "PyVISA-sim"
assert get_visa_backend("py") == "PyVISA-py"

_get_system_visa_info.cache_clear()
with mock.patch(
"subprocess.check_output",
mock.MagicMock(side_effect=SubprocessError()),
):
assert get_visa_backend("tests/sim_devices/devices.yaml") == "PyVISA-sim"
assert get_visa_backend("py") == "PyVISA-py"
finally:
# Reset the constant
tm_devices.helpers.functions._VISA_SYSTEM_DETAILS = old_system_details # noqa: SLF001
_get_system_visa_info.cache_clear()


@pytest.mark.parametrize(
Expand Down

0 comments on commit ce9d931

Please sign in to comment.