From 6ffa12ae5e08f5688ba92cce8a82619d83fef6c7 Mon Sep 17 00:00:00 2001 From: "Felt, Nicholas" Date: Thu, 7 Nov 2024 10:03:29 -0800 Subject: [PATCH 01/11] docs: Update link to badge --- README.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 0fb7d296..9b297b03 100644 --- a/README.md +++ b/README.md @@ -1,13 +1,13 @@
-| | | -| ----------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| **Testing** | [![Code testing status](https://github.com/tektronix/tm_devices/actions/workflows/test-code.yml/badge.svg?branch=main)](https://github.com/tektronix/tm_devices/actions/workflows/test-code.yml) [![Docs testing status](https://github.com/tektronix/tm_devices/actions/workflows/test-docs.yml/badge.svg?branch=main)](https://github.com/tektronix/tm_devices/actions/workflows/test-docs.yml) [![Coverage status](https://codecov.io/gh/tektronix/tm_devices/branch/main/graph/badge.svg)](https://codecov.io/gh/tektronix/tm_devices) | -| **Code Quality** | [![CodeQL status](https://github.com/tektronix/tm_devices/actions/workflows/codeql-analysis.yml/badge.svg?branch=main)](https://github.com/tektronix/tm_devices/actions/workflows/codeql-analysis.yml) [![CodeFactor grade](https://www.codefactor.io/repository/github/tektronix/tm_devices/badge)](https://www.codefactor.io/repository/github/tektronix/tm_devices) [![pre-commit status](https://results.pre-commit.ci/badge/github/tektronix/tm_devices/main.svg)](https://results.pre-commit.ci/latest/github/tektronix/tm_devices/main) | -| **Package** | [![PyPI: Package status](https://img.shields.io/pypi/status/tm_devices?logo=pypi)](https://pypi.org/project/tm_devices/) [![PyPI: Latest release version](https://img.shields.io/pypi/v/tm_devices?logo=pypi)](https://pypi.org/project/tm_devices/) [![PyPI: Supported Python versions](https://img.shields.io/pypi/pyversions/tm_devices?logo=python)](https://pypi.org/project/tm_devices/) [![PyPI: Downloads](https://pepy.tech/badge/tm-devices)](https://pepy.tech/project/tm_devices) [![License: Apache 2.0](https://img.shields.io/pypi/l/tm_devices)](https://github.com/tektronix/tm_devices/blob/main/LICENSE.md) [![Package build status](https://github.com/tektronix/tm_devices/actions/workflows/package-build.yml/badge.svg?branch=main)](https://github.com/tektronix/tm_devices/actions/workflows/package-build.yml) [![PyPI upload status](https://github.com/tektronix/tm_devices/actions/workflows/package-release.yml/badge.svg?branch=main)](https://github.com/tektronix/tm_devices/actions/workflows/package-release.yml) | -| **Documentation** | [![ReadtheDocs Status](https://img.shields.io/readthedocs/tm-devices/stable?logo=readthedocs)](https://tm-devices.readthedocs.io/stable) | -| **Code Style** | [![Test style: pytest](https://img.shields.io/badge/test%20style-pytest-blue)](https://github.com/pytest-dev/pytest) [![Code style: ruff](https://img.shields.io/badge/code%20style-ruff-black)](https://docs.astral.sh/ruff/formatter/) [![Docstring style: google](https://img.shields.io/badge/docstring%20style-google-tan)](https://google.github.io/styleguide/pyguide.html) | -| **Linting** | [![pre-commit enabled](https://img.shields.io/badge/pre--commit-enabled-brightgreen?logo=pre-commit)](https://github.com/pre-commit/pre-commit) [![Docstring formatter: docformatter](https://img.shields.io/badge/docstring%20formatter-docformatter-tan)](https://github.com/PyCQA/docformatter) [![Type Checker: pyright](https://img.shields.io/badge/type%20checker-pyright-yellowgreen)](https://github.com/RobertCraigie/pyright-python) [![Linter: pylint](https://img.shields.io/badge/linter-pylint-purple)](https://github.com/pylint-dev/pylint) [![Linter: Ruff](https://img.shields.io/badge/linter-ruff-purple)](https://github.com/charliermarsh/ruff) | +| | | +| ----------------- |-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| **Testing** | [![Code testing status](https://github.com/tektronix/tm_devices/actions/workflows/test-code.yml/badge.svg?branch=main)](https://github.com/tektronix/tm_devices/actions/workflows/test-code.yml) [![Docs testing status](https://github.com/tektronix/tm_devices/actions/workflows/test-docs.yml/badge.svg?branch=main)](https://github.com/tektronix/tm_devices/actions/workflows/test-docs.yml) [![Coverage status](https://codecov.io/gh/tektronix/tm_devices/branch/main/graph/badge.svg)](https://codecov.io/gh/tektronix/tm_devices) | +| **Code Quality** | [![CodeQL status](https://github.com/tektronix/tm_devices/actions/workflows/codeql-analysis.yml/badge.svg?branch=main)](https://github.com/tektronix/tm_devices/actions/workflows/codeql-analysis.yml) [![CodeFactor grade](https://www.codefactor.io/repository/github/tektronix/tm_devices/badge)](https://www.codefactor.io/repository/github/tektronix/tm_devices) [![pre-commit status](https://results.pre-commit.ci/badge/github/tektronix/tm_devices/main.svg)](https://results.pre-commit.ci/latest/github/tektronix/tm_devices/main) | +| **Package** | [![PyPI: Package status](https://img.shields.io/pypi/status/tm_devices?logo=pypi)](https://pypi.org/project/tm_devices/) [![PyPI: Latest release version](https://img.shields.io/pypi/v/tm_devices?logo=pypi)](https://pypi.org/project/tm_devices/) [![PyPI: Supported Python versions](https://img.shields.io/pypi/pyversions/tm_devices?logo=python)](https://pypi.org/project/tm_devices/) [![PyPI: Downloads](https://static.pepy.tech/badge/tm-devices)](https://pepy.tech/project/tm_devices) [![License: Apache 2.0](https://img.shields.io/pypi/l/tm_devices)](https://github.com/tektronix/tm_devices/blob/main/LICENSE.md) [![Package build status](https://github.com/tektronix/tm_devices/actions/workflows/package-build.yml/badge.svg?branch=main)](https://github.com/tektronix/tm_devices/actions/workflows/package-build.yml) [![PyPI upload status](https://github.com/tektronix/tm_devices/actions/workflows/package-release.yml/badge.svg?branch=main)](https://github.com/tektronix/tm_devices/actions/workflows/package-release.yml) | +| **Documentation** | [![ReadtheDocs Status](https://img.shields.io/readthedocs/tm-devices/stable?logo=readthedocs)](https://tm-devices.readthedocs.io/stable) | +| **Code Style** | [![Test style: pytest](https://img.shields.io/badge/test%20style-pytest-blue)](https://github.com/pytest-dev/pytest) [![Code style: ruff](https://img.shields.io/badge/code%20style-ruff-black)](https://docs.astral.sh/ruff/formatter/) [![Docstring style: google](https://img.shields.io/badge/docstring%20style-google-tan)](https://google.github.io/styleguide/pyguide.html) | +| **Linting** | [![pre-commit enabled](https://img.shields.io/badge/pre--commit-enabled-brightgreen?logo=pre-commit)](https://github.com/pre-commit/pre-commit) [![Docstring formatter: docformatter](https://img.shields.io/badge/docstring%20formatter-docformatter-tan)](https://github.com/PyCQA/docformatter) [![Type Checker: pyright](https://img.shields.io/badge/type%20checker-pyright-yellowgreen)](https://github.com/RobertCraigie/pyright-python) [![Linter: pylint](https://img.shields.io/badge/linter-pylint-purple)](https://github.com/pylint-dev/pylint) [![Linter: Ruff](https://img.shields.io/badge/linter-ruff-purple)](https://github.com/charliermarsh/ruff) |
From 5925bc803997f41c8370b8e5c4bd8b1b0a043f60 Mon Sep 17 00:00:00 2001 From: "Felt, Nicholas" Date: Thu, 7 Nov 2024 15:41:24 -0800 Subject: [PATCH 02/11] feat: Switched to using the Python logging module for printing out information to stdout. This provides users with more control over stdout and also allows for logging outputs to files. --- .gitignore | 1 + README.md | 2 +- pyproject.toml | 7 +- scripts/contributor_setup.py | 4 +- src/tm_devices/__init__.py | 6 +- .../commands/gen_e3e9uu_lpdmso/pilogger.py | 8 +- src/tm_devices/device_manager.py | 61 +++++--- ...bstract_device_visa_write_query_control.py | 8 +- .../device_control/pi_control.py | 144 +++++++++++------- .../device_control/rest_api_control.py | 78 +++++----- .../device_control/tsp_control.py | 9 +- src/tm_devices/drivers/device.py | 117 +++++++------- .../drivers/margin_testers/margin_tester.py | 2 - .../drivers/scopes/tekscope/tekscope.py | 5 + .../drivers/scopes/tekscope/tekscopepc.py | 10 +- .../drivers/scopes/tekscope_2k/tekscope_2k.py | 4 + src/tm_devices/helpers/__init__.py | 4 - src/tm_devices/helpers/enums.py | 17 +++ src/tm_devices/helpers/functions.py | 107 ++++++------- src/tm_devices/helpers/logging.py | 83 ++++++++++ src/tm_devices/helpers/singleton_metaclass.py | 10 +- .../helpers/verification_functions.py | 10 +- tests/conftest.py | 9 +- tests/test_all_device_drivers.py | 4 +- tests/test_config_parser.py | 4 +- tests/test_device_manager.py | 2 +- tests/test_helpers.py | 39 +---- tests/test_rest_api_device.py | 2 +- tests/test_scopes.py | 8 +- tests/test_singleton.py | 2 +- tests/test_smu.py | 9 +- tests/validate_device_manager_delete.py | 8 +- tests/verify_physical_device_support.py | 2 +- 33 files changed, 455 insertions(+), 331 deletions(-) create mode 100644 src/tm_devices/helpers/logging.py diff --git a/.gitignore b/.gitignore index d640ca09..dabcf6f0 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,7 @@ tm_devices.yaml tm_devices.yml *.log *.dat +**/logs/** # Poetry lock file poetry.lock diff --git a/README.md b/README.md index 9b297b03..6c7c397d 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@
| | | -| ----------------- |-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| ----------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | **Testing** | [![Code testing status](https://github.com/tektronix/tm_devices/actions/workflows/test-code.yml/badge.svg?branch=main)](https://github.com/tektronix/tm_devices/actions/workflows/test-code.yml) [![Docs testing status](https://github.com/tektronix/tm_devices/actions/workflows/test-docs.yml/badge.svg?branch=main)](https://github.com/tektronix/tm_devices/actions/workflows/test-docs.yml) [![Coverage status](https://codecov.io/gh/tektronix/tm_devices/branch/main/graph/badge.svg)](https://codecov.io/gh/tektronix/tm_devices) | | **Code Quality** | [![CodeQL status](https://github.com/tektronix/tm_devices/actions/workflows/codeql-analysis.yml/badge.svg?branch=main)](https://github.com/tektronix/tm_devices/actions/workflows/codeql-analysis.yml) [![CodeFactor grade](https://www.codefactor.io/repository/github/tektronix/tm_devices/badge)](https://www.codefactor.io/repository/github/tektronix/tm_devices) [![pre-commit status](https://results.pre-commit.ci/badge/github/tektronix/tm_devices/main.svg)](https://results.pre-commit.ci/latest/github/tektronix/tm_devices/main) | | **Package** | [![PyPI: Package status](https://img.shields.io/pypi/status/tm_devices?logo=pypi)](https://pypi.org/project/tm_devices/) [![PyPI: Latest release version](https://img.shields.io/pypi/v/tm_devices?logo=pypi)](https://pypi.org/project/tm_devices/) [![PyPI: Supported Python versions](https://img.shields.io/pypi/pyversions/tm_devices?logo=python)](https://pypi.org/project/tm_devices/) [![PyPI: Downloads](https://static.pepy.tech/badge/tm-devices)](https://pepy.tech/project/tm_devices) [![License: Apache 2.0](https://img.shields.io/pypi/l/tm_devices)](https://github.com/tektronix/tm_devices/blob/main/LICENSE.md) [![Package build status](https://github.com/tektronix/tm_devices/actions/workflows/package-build.yml/badge.svg?branch=main)](https://github.com/tektronix/tm_devices/actions/workflows/package-build.yml) [![PyPI upload status](https://github.com/tektronix/tm_devices/actions/workflows/package-release.yml/badge.svg?branch=main)](https://github.com/tektronix/tm_devices/actions/workflows/package-release.yml) | diff --git a/pyproject.toml b/pyproject.toml index 1bfa03ea..ecdb75f1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -76,6 +76,7 @@ repository = "https://github.com/tektronix/tm_devices" version = "2.5.0" [tool.poetry.dependencies] +colorlog = "^6.9.0" gpib-ctypes = "^0.3.0" libusb-package = "^1.0.26.0,!=1.0.26.2" # 1.0.26.2 doesn't work with Python 3.12 packaging = "^24.0" @@ -94,6 +95,7 @@ tomli = "^2.0.1" tomli-w = "^1.0.0" traceback-with-variables = "^2.0.4" typing-extensions = "^4.10.0" +tzlocal = "^5.2" urllib3 = "^2.0" zeroconf = "^0.136.0" @@ -235,6 +237,7 @@ disable = [ "too-many-lines", # not necessary to check for "too-many-statements", # caught by ruff "too-many-statements", # caught by ruff + "unexpected-keyword-arg", # caught by pyright "unused-argument", # caught by ruff "unused-import", # caught by ruff "use-implicit-booleaness-not-comparison-to-string", # caught by ruff @@ -328,7 +331,6 @@ ignore = [ "PTH123", # `open()` should be replaced by `Path.open()` "PTH207", # Replace `iglob` with `Path.glob` or `Path.rglob` "PYI021", # Docstrings should not be included in stubs - "T20", # flake8-print "TD002", # Missing author in TO DO "TD003", # Missing issue link on the line following this TO DO "TRY301", # Abstract raise to an inner function @@ -357,7 +359,8 @@ order-by-type = false [tool.ruff.lint.per-file-ignores] "examples/**" = [ - "S101" # Use of assert detected + "S101", # Use of assert detected + "T201" # `print` found ] "examples/miscellaneous/custom_device_driver_support.py" = [ "ARG002", # Unused method argument diff --git a/scripts/contributor_setup.py b/scripts/contributor_setup.py index bcb29daf..fd38579f 100644 --- a/scripts/contributor_setup.py +++ b/scripts/contributor_setup.py @@ -24,7 +24,7 @@ def create_virtual_environment(virtual_env_dir: str | os.PathLike[str]) -> None: Args: virtual_env_dir: The directory where the virtual environment should be created """ - print(f"\nCreating virtualenv located at '{virtual_env_dir}'") + print(f"\nCreating virtualenv located at '{virtual_env_dir}'") # noqa: T201 _run_cmd_in_subprocess(f'"{sys.executable}" -m venv "{virtual_env_dir}" --clear') @@ -35,7 +35,7 @@ def _run_cmd_in_subprocess(command: str) -> None: command: The command string to send. """ command = command.replace("\\", "/") - print(f"\nExecuting command: {command}") + print(f"\nExecuting command: {command}") # noqa: T201 subprocess.check_call(shlex.split(command)) # noqa: S603 diff --git a/src/tm_devices/__init__.py b/src/tm_devices/__init__.py index e4e4cd00..36248516 100644 --- a/src/tm_devices/__init__.py +++ b/src/tm_devices/__init__.py @@ -19,14 +19,18 @@ PYVISA_PY_BACKEND, SYSTEM_DEFAULT_VISA_BACKEND, ) -from tm_devices.helpers.enums import SupportedModels +from tm_devices.helpers.enums import LoggingLevels, SupportedModels from tm_devices.helpers.functions import register_additional_usbtmc_mapping +from tm_devices.helpers.logging import configure_logging # Read version from installed package. __version__ = version(PACKAGE_NAME) + __all__ = [ + "configure_logging", "DeviceManager", + "LoggingLevels", "print_available_visa_devices", "PYVISA_PY_BACKEND", "register_additional_usbtmc_mapping", diff --git a/src/tm_devices/commands/gen_e3e9uu_lpdmso/pilogger.py b/src/tm_devices/commands/gen_e3e9uu_lpdmso/pilogger.py index c658948a..37f84b2b 100644 --- a/src/tm_devices/commands/gen_e3e9uu_lpdmso/pilogger.py +++ b/src/tm_devices/commands/gen_e3e9uu_lpdmso/pilogger.py @@ -47,8 +47,8 @@ class PiloggerState(SCPICmdWrite, SCPICmdRead): Info: - ```` = 0 disables the PI command logger; any other value turns the PI command logger on. - - ``OFF`` disables the PI command logger. - - ``ON`` enables the PI command logger. + - ``OFF`` disables the PI command _logger. + - ``ON`` enables the PI command _logger. """ @@ -152,7 +152,7 @@ def state(self) -> PiloggerState: Info: - ```` = 0 disables the PI command logger; any other value turns the PI command logger on. - - ``OFF`` disables the PI command logger. - - ``ON`` enables the PI command logger. + - ``OFF`` disables the PI command _logger. + - ``ON`` enables the PI command _logger. """ return self._state diff --git a/src/tm_devices/device_manager.py b/src/tm_devices/device_manager.py index c5fa1b4f..8d0cbc6d 100644 --- a/src/tm_devices/device_manager.py +++ b/src/tm_devices/device_manager.py @@ -6,6 +6,7 @@ import contextlib import inspect import json +import logging import os import pathlib import socket @@ -44,7 +45,6 @@ DMConfigOptions, get_model_series, PACKAGE_NAME, - print_with_timestamp, PYVISA_PY_BACKEND, SerialConfig, Singleton, @@ -98,6 +98,8 @@ UnsupportedDeviceAlias = TypeVar("UnsupportedDeviceAlias", bound=Device, default=Device) """An alias to a custom device driver for an unsupported device type.""" +_logger: logging.Logger = logging.getLogger(__name__) + #################################################################################################### # DeviceManager class @@ -124,11 +126,20 @@ def __init__( """Create the instance of the DeviceManager. Args: - verbose: A boolean indicating if verbose output should be printed. + verbose: A boolean indicating if verbose output should be printed. This flag cascades + down to all connected devices. Setting it to False **will not** remove all printouts + to stdout. To remove all console output, use + [`configure_logging()`][tm_devices.configure_logging] before instantiating the + DeviceManager. config_options: An optional set of configuration options to use (updates the current configuration options). external_device_drivers: An optional dict for passing in additional device drivers. """ + # pylint: disable=import-outside-toplevel + if not logging.getLogger(PACKAGE_NAME).hasHandlers(): + from tm_devices.helpers.logging import configure_logging + + configure_logging() self._suppress_protection = False # Set up the DeviceManager self.__is_open = False @@ -683,21 +694,20 @@ def cleanup_all_devices(self) -> None: try: device_object.cleanup(verbose=bool(self.__config.options.verbose_mode)) except (visa.errors.Error, socket.error, RPCError, AttributeError): # noqa: PERF203 - print(f"Device cleanup of {device_name} failed. Retrying...") + _logger.warning("Device cleanup of %s failed. Retrying...", device_name) device_object.cleanup() def close(self) -> None: """Close the DeviceManager.""" self.__protect_access() - print() if self.__devices: - print_with_timestamp("Closing Connections to Devices") + _logger.info("Closing Connections to Devices") if self.__teardown_cleanup_enabled: self.cleanup_all_devices() for device_object in list(self.__devices.values()): device_object.close() self.__is_open = False - print_with_timestamp(f"{self.__class__.__name__} Closed") + _logger.info("%s Closed", self.__class__.__name__) def disable_device_command_checking(self) -> None: """Set the `.enable_verification` attribute of each device to `False`. @@ -1007,13 +1017,11 @@ def load_config_file(self, config_file_path: Union[str, os.PathLike[str]]) -> No Args: config_file_path: The path to the config file to load. """ - print_with_timestamp( - f"Loading Configuration from {pathlib.Path(config_file_path).resolve()}" - ) + _logger.info("Loading Configuration from %s", pathlib.Path(config_file_path).resolve()) self.__config.load_config_file(config_file_path) self.__set_options(self.__verbose) if len(self.__config.devices) != len(self.__devices): - print_with_timestamp("Opening Connections to Devices") + _logger.info("Opening Connections to Devices") for device_name, device_config in self.__config.devices.items(): if device_name not in self.__devices: self.__create_device(device_name, device_config, 3) @@ -1031,10 +1039,10 @@ def open(self) -> bool: f"{self.__class__.__name__} has already been closed." ) raise AssertionError(msg) - print_with_timestamp(f"Opening {self.__class__.__name__}") + _logger.info("Opening %s", self.__class__.__name__) # Create the devices if self.__config.devices: - print_with_timestamp("Opening Connections to Devices") + _logger.info("Opening Connections to Devices") for device_name, device_config in self.__config.devices.items(): self.__create_device(device_name, device_config, 3) if self.__setup_cleanup_enabled: @@ -1093,9 +1101,9 @@ def write_current_configuration_to_config_file( Args: config_file_path: The path to the config file. If ends in ".toml" will create toml file. """ - print_with_timestamp("Writing Configuration to file") + _logger.info("Writing Configuration to file") new_file_path = pathlib.Path(self.__config.write_config_to_file(config_file_path)).resolve() - print_with_timestamp(f"Wrote Configuration to {new_file_path}") + _logger.info("Wrote Configuration to %s", new_file_path) def get_current_configuration_as_environment_variable_strings(self) -> str: """Return the current configuration represented as environment variables.""" @@ -1191,23 +1199,25 @@ def __clear_visa_output_buffer_and_get_idn(visa_resource: MessageBasedResource) # 16 is the MAV bit (Message Available) from the Status Byte register # MAV flag is only one bit and turns off after a single response is # successfully read, even if there is more in the buffer. - warnings.warn( + msg = ( f"\nThe device `{visa_resource.resource_info.resource_name}` had data " "sitting in the VISA Output Buffer on first connection. " "\nDetected data in the buffer via the Status Byte register. " - "\nThe device_clear() will be called so VISA I/O buffers get flushed.", - stacklevel=1, + "\nThe device_clear() will be called so VISA I/O buffers get flushed." ) + warnings.warn(msg, stacklevel=1) + _logger.warning(msg) # always flush the VISA I/O Buffers on the device to clean up any stale data. # (note: the Events are kept in different buffers, so *ESR? is not impacted) visa_resource.clear() except visa.VisaIOError as e: - warnings.warn( + msg = ( f"A VISA IO error occurred when attempting to read the status byte or clear the " f"output buffer of the resource `{visa_resource.resource_info.resource_name}`.\n" - f"Error: {e}", - stacklevel=1, + f"Error: {e}" ) + warnings.warn(msg, stacklevel=1) + _logger.warning(msg) visa_resource.write("*IDN?") idn_response = "" error_msg = None @@ -1271,14 +1281,15 @@ def __create_device( alias_string = f' "{device_config.alias}"' if device_config.alias else "" if device_config.device_type == DeviceTypes.UNSUPPORTED: - warnings.warn( + msg = ( f"An unsupported device type is being added to the {self.__class__.__name__}. " f"Not all functionality will be available in the device driver. " f"Please consider contributing to {PACKAGE_NAME} to implement official " - f"support for this device type.", - stacklevel=warning_stacklevel, + f"support for this device type." ) - print_with_timestamp(f"Creating Connection to {device_config_name}{alias_string}") + warnings.warn(msg, stacklevel=warning_stacklevel) + _logger.warning(msg) + _logger.info("Creating Connection to %s%s", device_config_name, alias_string) new_device: Device if device_config.connection_type == ConnectionTypes.REST_API: device_driver_class = device_drivers[str(device_config.device_driver)] @@ -1436,4 +1447,4 @@ def print_available_visa_devices() -> None: # pragma: no cover """Print all available VISA devices to the console.""" with contextlib.redirect_stdout(None), contextlib.redirect_stderr(None), DeviceManager() as dm: available_devices = dm.get_available_devices() - print(json.dumps(available_devices["local"], indent=2)) + print(json.dumps(available_devices["local"], indent=2)) # noqa: T201 diff --git a/src/tm_devices/driver_mixins/device_control/_abstract_device_visa_write_query_control.py b/src/tm_devices/driver_mixins/device_control/_abstract_device_visa_write_query_control.py index bf1a31fb..5676ce36 100644 --- a/src/tm_devices/driver_mixins/device_control/_abstract_device_visa_write_query_control.py +++ b/src/tm_devices/driver_mixins/device_control/_abstract_device_visa_write_query_control.py @@ -1,5 +1,7 @@ """A class defining methods that VISA devices and control mixins must have.""" +import logging + from abc import abstractmethod from typing import Any, final, Tuple @@ -8,6 +10,8 @@ ) from tm_devices.helpers import raise_failure, verify_values +_logger: logging.Logger = logging.getLogger(__name__) + class _AbstractDeviceVISAWriteQueryControl(_AbstractDeviceControl): # pyright: ignore[reportUnusedClass] """Abstract class defining methods that VISA devices and control mixins must have.""" @@ -64,7 +68,7 @@ def expect_esr( ) except AssertionError as exc: check_passed &= False - print(exc) # the exception already contains the timestamp + _logger.warning(exc) # Compare the error messages for expected_message, actual_message in zip(error_messages, actual_error_messages): @@ -79,7 +83,7 @@ def expect_esr( ) except AssertionError as exc: # noqa: PERF203 check_passed &= False - print(exc) + _logger.warning(exc) if not check_passed: failure_message = ( diff --git a/src/tm_devices/driver_mixins/device_control/pi_control.py b/src/tm_devices/driver_mixins/device_control/pi_control.py index 2d3249b2..0c1793d3 100644 --- a/src/tm_devices/driver_mixins/device_control/pi_control.py +++ b/src/tm_devices/driver_mixins/device_control/pi_control.py @@ -1,5 +1,6 @@ """Base Programmable Interface (PI) control class module.""" +import logging import os import socket import time @@ -29,13 +30,14 @@ get_model_series, get_version, get_visa_backend, - print_with_timestamp, PYVISA_PY_BACKEND, raise_failure, verify_values, ) from tm_devices.helpers import ReadOnlyCachedProperty as cached_property # noqa: N813 +_logger: logging.Logger = logging.getLogger(__name__) + class PIControl(_AbstractDeviceVISAWriteQueryControl, _ExtendableMixin, ABC): # pylint: disable=too-many-public-methods """Base Programmable Interface (PI) control class. @@ -122,8 +124,7 @@ def visa_timeout(self, timeout_ms: float) -> None: timeout_ms: The new VISA timeout, in milliseconds. """ self._visa_resource.timeout = timeout_ms - if self._verbose: - print(f"{self._name_and_alias} VISA timeout set to: {timeout_ms}ms") + _logger.debug("%s VISA timeout set to: %fms", self._name_and_alias, timeout_ms) @property def visa_resource(self) -> visa.resources.MessageBasedResource: @@ -210,19 +211,15 @@ def temporary_visa_timeout(self, temporary_timeout_ms: float) -> Generator[None, # Public Methods ################################################################################################ @final - def check_visa_connection(self, verbose: bool = True) -> bool: + def check_visa_connection(self) -> bool: """Check if a VISA connection can be made to the device. Wrapper function for [`check_visa_connection`][tm_devices.helpers.check_visa_connection]. - - Args: - verbose: Set this to False in order to disable printouts. """ return check_visa_connection( self._config_entry, self._visa_library_path, self._name_and_alias, - verbose=verbose and self._verbose, ) def device_clear(self) -> None: # pragma: no cover @@ -340,8 +337,13 @@ def query( # pylint: disable=arguments-differ Error: An error occurred while sending the command. SystemError: An empty string was returned from the device. """ - if self._verbose and verbose: - print_with_timestamp(f"({self._name_and_alias}) Query >> {query!r}") + # TODO: logging: Log just the query to a device specific file + _logger.log( + logging.INFO if self._verbose and verbose else logging.DEBUG, + "(%s) Query >> %r", + self._name_and_alias, + query, + ) try: response = self._visa_resource.query(query).strip() @@ -352,8 +354,12 @@ def query( # pylint: disable=arguments-differ msg = f"The query{pi_cmd_repr}failed with the following message: {error!r}" raise visa.Error(msg) from error - if self._verbose and verbose: - print_with_timestamp(f"Response from {query!r} >> {response!r}") + _logger.log( + logging.INFO if self._verbose and verbose else logging.DEBUG, + "Response from %r >> %r", + query, + response, + ) if not allow_empty and not response: pi_cmd_repr = ( @@ -378,8 +384,13 @@ def query_binary(self, query: str, verbose: bool = True) -> Sequence[float]: Error: An error occurred while sending the command. SystemError: An empty string was returned from the device. """ - if self._verbose and verbose: - print_with_timestamp(f"({self._name_and_alias}) Query Binary Values >> {query!r}") + # TODO: logging: Log just the query to a device specific file + _logger.log( + logging.INFO if self._verbose and verbose else logging.DEBUG, + "(%s) Query Binary Values >> %r", + self._name_and_alias, + query, + ) try: response = self._visa_resource.query_binary_values(query) # pyright: ignore[reportUnknownMemberType] @@ -388,10 +399,12 @@ def query_binary(self, query: str, verbose: bool = True) -> Sequence[float]: msg = f"The query{pi_cmd_repr}failed with the following message: {error!r}" raise visa.Error(msg) from error - if self._verbose and verbose: - print_with_timestamp( - f"Response from {query!r} >> {', '.join([str(x) for x in response])!r}" - ) + _logger.log( + logging.INFO if self._verbose and verbose else logging.DEBUG, + "Response from %r >> %r", + query, + response, + ) if not response: pi_cmd_repr = ( @@ -501,8 +514,13 @@ def query_raw_binary(self, query: str, verbose: bool = True) -> bytes: Error: An error occurred while sending the command. SystemError: An empty string was returned from the device. """ - if self._verbose and verbose: - print_with_timestamp(f"({self._name_and_alias}) Query Raw Binary >> {query!r}") + # TODO: logging: Log just the query to a device specific file + _logger.log( + logging.INFO if self._verbose and verbose else logging.DEBUG, + "(%s) Query Raw Binary >> %r", + self._name_and_alias, + query, + ) try: self._visa_resource.write(query) @@ -512,8 +530,12 @@ def query_raw_binary(self, query: str, verbose: bool = True) -> bytes: msg = f"The query{pi_cmd_repr}failed with the following message: {error!r}" raise visa.Error(msg) from error - if self._verbose and verbose: - print_with_timestamp(f"Response from {query!r} >> {response!r}") + _logger.log( + logging.INFO if self._verbose and verbose else logging.DEBUG, + "Response from %r >> %r", + query, + response, + ) if not response.strip(): pi_cmd_repr = ( @@ -768,13 +790,14 @@ def wait_for_visa_connection( """ attempt_num = 0 visa_connection = False - if verbose: - print_with_timestamp( - f"Attempting to establish a VISA connection with {self._resource_expression}" - ) + _logger.log( + logging.INFO if verbose else logging.DEBUG, + "Attempting to establish a VISA connection with %s", + self._resource_expression, + ) start_time = time.perf_counter() while (time.perf_counter() - start_time) <= wait_time: - if visa_connection := self.check_visa_connection(verbose=False): + if visa_connection := self.check_visa_connection(): # pylint: disable=compare-to-zero if not (attempt_num == 0 and not accept_immediate_connection): break @@ -790,17 +813,19 @@ def wait_for_visa_connection( end_time = time.perf_counter() total_time = end_time - start_time - if verbose: - if visa_connection: - print_with_timestamp( - f"Successfully established a VISA connection with {self._resource_expression} " - f"after {total_time:.2f} seconds" - ) - else: - print_with_timestamp( - f"Unable to establish a VISA connection with {self._resource_expression} " - f"after {total_time:.2f} seconds" - ) + if visa_connection: + _logger.log( + logging.INFO if verbose else logging.DEBUG, + "Successfully established a VISA connection with %s after %.2f seconds", + self._resource_expression, + total_time, + ) + else: + _logger.warning( + "Unable to establish a VISA connection with %s after %.2f seconds", + self._resource_expression, + total_time, + ) return visa_connection @@ -816,19 +841,21 @@ def write(self, command: str, opc: bool = False, verbose: bool = True) -> None: Error: An error occurred while sending the command. SystemError: ``*OPC?`` did not return "1" after sending the command. """ - if self._verbose and verbose: - if "\n" in command: - # Format any multiline command to print out with a single timestamp - # followed by as many (whitespace padded) f">> {cmd}" lines as it has - commands_iter = iter(repr(command.strip()).split("\\n")) - spaces = " " * len( - print_with_timestamp( - f"({self._name_and_alias}) Write >> {next(commands_iter)}" - ).split(">> ")[0] - ) - print(*[f"{spaces}>> {cmd}" for cmd in commands_iter], sep="\n") - else: - print_with_timestamp(f"({self._name_and_alias}) Write >> {command!r}") + # TODO: logging: Log just the write to a device specific file + if "\n" in command: + _logger.log( + logging.INFO if self._verbose and verbose else logging.DEBUG, + "(%s) Write >>\n%s", + self._name_and_alias, + command, + ) + else: + _logger.log( + logging.INFO if self._verbose and verbose else logging.DEBUG, + "(%s) Write >> %r", + self._name_and_alias, + command, + ) try: self._visa_resource.write(command) @@ -852,8 +879,13 @@ def write_raw(self, command: bytes, verbose: bool = True) -> None: Raises: Error: An error occurred while sending the command. """ - if self._verbose and verbose: - print_with_timestamp(f"({self._name_and_alias}) Write Raw >> {command!r}") + # TODO: logging: Log just the write to a device specific file + _logger.log( + logging.INFO if self._verbose and verbose else logging.DEBUG, + "(%s) Write Raw >> %r", + self._name_and_alias, + command, + ) try: self._visa_resource.write_raw(command) @@ -918,9 +950,9 @@ def _close(self) -> None: try: self._visa_resource.close() except VisaIOError as error: - warnings.warn( - f"Error encountered while closing the visa resource:\n{error}", stacklevel=2 - ) + error_msg = f"Error encountered while closing the visa resource:\n{error}" + warnings.warn(error_msg, stacklevel=2) + _logger.exception(error_msg) self._visa_resource = None # pyright: ignore[reportAttributeAccessIssue] self._is_open = False diff --git a/src/tm_devices/driver_mixins/device_control/rest_api_control.py b/src/tm_devices/driver_mixins/device_control/rest_api_control.py index d103a3d7..4c8fe3d2 100644 --- a/src/tm_devices/driver_mixins/device_control/rest_api_control.py +++ b/src/tm_devices/driver_mixins/device_control/rest_api_control.py @@ -1,6 +1,7 @@ """Base REST Application Programming Interface (API) control class module.""" import json +import logging import time from abc import ABC, abstractmethod @@ -14,11 +15,12 @@ ) from tm_devices.helpers import ( DeviceConfigEntry, - print_with_timestamp, raise_failure, SupportedRequestTypes, ) +_logger: logging.Logger = logging.getLogger(__name__) + class RESTAPIControl(_AbstractDeviceControl, ABC): """Base REST Application Programming Interface (API) control class. @@ -307,23 +309,25 @@ def put( # noqa: PLR0913 verbose=verbose, ) - def set_api_version(self, api_version: int, verbose: bool = True) -> None: + def set_api_version(self, api_version: int) -> None: """Set the API version used by the device. Args: api_version: The API version to use for this device - verbose: Set this to False in order to disable printouts. """ self._api_url = self._base_url + self.API_VERSIONS[api_version] - if self._verbose and verbose: - print(f"{self._name_and_alias} API version set to: {api_version} ({self._api_url})") + _logger.debug( + "%s API version set to: %d (%s)", + self._name_and_alias, + api_version, + self._api_url, + ) def wait_for_api_connection( self, wait_time: float, sleep_seconds: int = 5, accept_immediate_connection: bool = False, - verbose: bool = True, ) -> bool: """Wait for an API call to go through to the device. @@ -334,7 +338,6 @@ def wait_for_api_connection( sleep_seconds: The number of seconds to sleep in between connection attempts. accept_immediate_connection: A boolean indicating if a connection on the first attempt is a valid connection. - verbose: Set this to False in order to disable printouts. Returns: A boolean indicating if an API connection was made within the given time limit. @@ -344,8 +347,11 @@ def wait_for_api_connection( """ attempt_num = 0 api_connection = False - if verbose: - print_with_timestamp(f"Attempting to establish an API connection with {self._api_url}") + _logger.log( + logging.INFO if self._verbose else logging.DEBUG, + "Attempting to establish an API connection with %s", + self._api_url, + ) start_time = time.perf_counter() while (time.perf_counter() - start_time) <= wait_time: if api_connection := self._check_api_connection(): @@ -364,17 +370,19 @@ def wait_for_api_connection( end_time = time.perf_counter() total_time = end_time - start_time - if verbose: - if api_connection: - print_with_timestamp( - f"Successfully established an API connection with {self._api_url} " - f"after {total_time:.2f} seconds" - ) - else: - print_with_timestamp( - f"Unable to establish an API connection with {self._api_url} " - f"after {total_time:.2f} seconds" - ) + if api_connection: + _logger.log( + logging.INFO if self._verbose else logging.DEBUG, + "Successfully established an API connection with %s after %.2f seconds", + self._api_url, + total_time, + ) + else: + _logger.warning( + "Unable to establish an API connection with %s after %.2f seconds", + self._api_url, + total_time, + ) return api_connection ################################################################################################ @@ -441,13 +449,15 @@ def _send_request( # noqa: PLR0913,PLR0912,C901 url = self._api_url + url response = cast(requests.Response, None) retval: Union[Dict[str, Any], bytes] = {} - if self._verbose and verbose: - print_with_timestamp(f"({self._name_and_alias}) {request_type.value} >> {url}", end="") - if headers: - print(f", {headers=}", end="") - if json_body: - print(f", {json_body=}", end="") - print() + _logger.log( + logging.INFO if verbose else logging.DEBUG, + "(%s) %s >> %s%s%s", + self._name_and_alias, + request_type.value, + url, + f", {headers=}" if headers else "", + f", {json_body=}" if json_body else "", + ) try: if request_type == SupportedRequestTypes.DELETE: response = requests.delete( @@ -503,13 +513,13 @@ def _send_request( # noqa: PLR0913,PLR0912,C901 else: msg = f"{request_type} is an unsupported request type." raise ValueError(msg) - if self._verbose and verbose: - print_with_timestamp(f"Response from {request_type.value} >>") - if not return_bytes: - print(f"{json.dumps(response.json(), indent=2)}") - else: - # Print .zip files as byte strings, response.content.decode() throws errors - print(f"\n{response.text}") + + _logger.log( + logging.INFO if verbose else logging.DEBUG, + "Response from %s >>\n%s", + request_type.value, + json.dumps(response.json(), indent=2) if not return_bytes else response.text, + ) retval = response.content if return_bytes else response.json() # If the response was successful, no Exception will be raised response.raise_for_status() diff --git a/src/tm_devices/driver_mixins/device_control/tsp_control.py b/src/tm_devices/driver_mixins/device_control/tsp_control.py index 7cca6753..e632f836 100644 --- a/src/tm_devices/driver_mixins/device_control/tsp_control.py +++ b/src/tm_devices/driver_mixins/device_control/tsp_control.py @@ -2,6 +2,8 @@ from __future__ import annotations +import logging + from abc import ABC from typing import Any, Dict, List, Optional, TYPE_CHECKING, Union @@ -13,6 +15,9 @@ import os +_logger: logging.Logger = logging.getLogger(__name__) + + class TSPControl(PIControl, ABC): """Base Test Script Processing (TSP) control class. @@ -164,9 +169,9 @@ def print_buffers(self, *args: str) -> None: def fix_width(key: str, value: Any) -> str: # Function to add spaces if needed return str(value) + " " * (column_widths[key] - len(str(value))) - print(*[fix_width(x, x) for x in buffer_data]) + print(*[fix_width(x, x) for x in buffer_data]) # noqa: T201 for index in range(column_length): - print( + print( # noqa: T201 *[fix_width(k, v[index] if index < len(v) else "") for k, v in buffer_data.items()] ) diff --git a/src/tm_devices/drivers/device.py b/src/tm_devices/drivers/device.py index 20f9f326..f1156e0f 100644 --- a/src/tm_devices/drivers/device.py +++ b/src/tm_devices/drivers/device.py @@ -1,6 +1,7 @@ """Base device driver module.""" import concurrent.futures +import logging import socket import time @@ -29,13 +30,14 @@ check_port_connection, ConnectionTypes, DeviceConfigEntry, - print_with_timestamp, ) from tm_devices.helpers import ReadOnlyCachedProperty as cached_property # noqa: N813 _T = TypeVar("_T") _FAMILY_BASE_CLASS_PROPERTY_NAME = "_product_family_base_class" +_logger: logging.Logger = logging.getLogger(__name__) + def family_base_class(cls: _T) -> _T: """A decorator indicating a class is the top level of a unique product family tree. @@ -362,27 +364,20 @@ def temporary_verbose(self, temporary_verbose: bool) -> Generator[None, None, No # Public Methods ################################################################################################ @final - def check_network_connection(self, verbose: bool = True) -> Tuple[bool, str]: + def check_network_connection(self) -> Tuple[bool, str]: """Check the network connection to the device using an external ping command. Wrapper function for [`check_network_connection`][tm_devices.helpers.check_network_connection]. - Args: - verbose: Set this to False in order to disable printouts. - Returns: A tuple containing a boolean indicating if there is a network connection and a string with the result of the ping command. """ - return check_network_connection( - self._name_and_alias, self.ip_address, verbose=verbose and self._verbose - ) + return check_network_connection(self._name_and_alias, self.ip_address) @final - def check_port_connection( - self, port: int, timeout_seconds: int = 5, verbose: bool = True - ) -> bool: + def check_port_connection(self, port: int, timeout_seconds: int = 5) -> bool: """Check if the given port is open on the device. Wrapper function for [`check_port_connection`][tm_devices.helpers.check_port_connection]. @@ -390,7 +385,6 @@ def check_port_connection( Args: port: The port to check. timeout_seconds: The number of seconds to use as the socket timeout. - verbose: Set this to False in order to disable printouts. Returns: A boolean indicating if the port is open. @@ -400,7 +394,6 @@ def check_port_connection( self.ip_address, port, timeout_seconds=timeout_seconds, - verbose=verbose and self._verbose, ) @final @@ -411,14 +404,14 @@ def cleanup(self, verbose: bool = True) -> None: verbose: Set this to False in order to disable printouts. """ with self.temporary_verbose(verbose): - print(f"Beginning Device Cleanup on {self._name_and_alias}") + _logger.info("Beginning Device Cleanup on %s", self._name_and_alias) self._cleanup() - print(f"Finished Device Cleanup on {self._name_and_alias}") + _logger.info("Finished Device Cleanup on %s", self._name_and_alias) @final def close(self) -> None: """Close this device and all its used resources and components.""" - print_with_timestamp(f"Closing Connection to {self._name_and_alias}") + _logger.info("Closing Connection to %s", self._name_and_alias) self._close() @final @@ -467,21 +460,19 @@ def reboot(self, quiet_period: int = 0) -> bool: self.__delattr__(prop) # pylint: disable=unnecessary-dunder-call # Reboot the device - print_with_timestamp(f"Rebooting {self._name_and_alias}") + _logger.info("Rebooting %s", self._name_and_alias) self._reboot() self.close() # Depending on the instrument model, shutdown time or reboot time can take longer if quiet_period or self._reboot_quiet_period: sleep_time = max(self._reboot_quiet_period, quiet_period) - print_with_timestamp(f"Waiting for {sleep_time} seconds") + _logger.info("Waiting for %d seconds", sleep_time) time.sleep(sleep_time) - print_with_timestamp(f"Reopening Connection to {self._name_and_alias}") + _logger.info("Reopening Connection to %s", self._name_and_alias) if rebooted := self._open(): - print_with_timestamp(f"Connection Reestablished with {self._name_and_alias}") + _logger.info("Connection Reestablished with %s", self._name_and_alias) else: - print_with_timestamp( - f"Failed to reestablish the connection with {self._name_and_alias}" - ) + _logger.error("Failed to reestablish the connection with %s", self._name_and_alias) return rebooted @final @@ -490,7 +481,6 @@ def wait_for_network_connection( wait_time: float, sleep_seconds: int = 2, accept_immediate_connection: bool = False, - verbose: bool = True, ) -> bool: """Wait for a network connection to the device. @@ -499,7 +489,6 @@ def wait_for_network_connection( sleep_seconds: The number of seconds to sleep in between connection attempts. accept_immediate_connection: A boolean indicating if a connection on the first attempt is a valid connection. - verbose: Set this to False in order to disable printouts. Returns: A boolean indicating if a network connection was made within the given time limit. @@ -509,13 +498,14 @@ def wait_for_network_connection( """ attempt_num = 0 network_connection = False - if verbose: - print_with_timestamp( - f"Attempting to establish a network connection with {self.ip_address}" - ) + _logger.log( + logging.INFO if self._verbose else logging.DEBUG, + "Attempting to establish a network connection with %s", + self.ip_address, + ) start_time = time.perf_counter() while (time.perf_counter() - start_time) <= wait_time: - if network_connection := self.check_network_connection(verbose=False)[0]: + if network_connection := self.check_network_connection()[0]: # pylint: disable=compare-to-zero if not (attempt_num == 0 and not accept_immediate_connection): break @@ -531,17 +521,19 @@ def wait_for_network_connection( end_time = time.perf_counter() total_time = end_time - start_time - if verbose: - if network_connection: - print_with_timestamp( - f"Successfully established a network connection with {self.ip_address} " - f"after {total_time:.2f} seconds" - ) - else: - print_with_timestamp( - f"Unable to establish a network connection with {self.ip_address} " - f"after {total_time:.2f} seconds" - ) + if network_connection: + _logger.log( + logging.INFO if self._verbose else logging.DEBUG, + "Successfully established a network connection with %s after %.2f seconds", + self.ip_address, + total_time, + ) + else: + _logger.warning( + "Unable to establish a network connection with %s after %.2f seconds", + self.ip_address, + total_time, + ) return network_connection def wait_for_port_connection( @@ -550,7 +542,6 @@ def wait_for_port_connection( wait_time: float, sleep_seconds: int = 5, accept_immediate_connection: bool = False, - verbose: bool = True, ) -> bool: """Wait for a connection to be made to the given port on the device. @@ -560,7 +551,6 @@ def wait_for_port_connection( sleep_seconds: The number of seconds to sleep in between connection attempts. accept_immediate_connection: A boolean indicating if a connection on the first attempt is a valid connection. - verbose: Set this to False in order to disable printouts. Returns: A boolean indicating if a connection was made to the port within the given time limit. @@ -570,16 +560,15 @@ def wait_for_port_connection( """ attempt_num = 0 port_connection = False - if verbose: - print_with_timestamp( - f"Attempting to establish a connection to port {port} on {self.ip_address}" - ) + _logger.log( + logging.INFO if self._verbose else logging.DEBUG, + "Attempting to establish a connection to port %d on %s", + port, + self.ip_address, + ) start_time = time.perf_counter() while (time.perf_counter() - start_time) <= wait_time: - port_connection = self.check_port_connection( - port, timeout_seconds=sleep_seconds, verbose=False - ) - if port_connection: + if port_connection := self.check_port_connection(port, timeout_seconds=sleep_seconds): # pylint: disable=compare-to-zero if not (attempt_num == 0 and not accept_immediate_connection): break @@ -595,17 +584,21 @@ def wait_for_port_connection( end_time = time.perf_counter() total_time = end_time - start_time - if verbose: - if port_connection: - print_with_timestamp( - f"Successfully established a connection to port {port} on {self.ip_address} " - f"after {total_time:.2f} seconds" - ) - else: - print_with_timestamp( - f"Unable to establish a connection to port {port} on {self.ip_address} " - f"after {total_time:.2f} seconds" - ) + if port_connection: + _logger.log( + logging.INFO if self._verbose else logging.DEBUG, + "Successfully established a connection to port %d on %s after %.2f seconds", + port, + self.ip_address, + total_time, + ) + else: + _logger.warning( + "Unable to establish a connection to port %d on %s after %.2f seconds", + port, + self.ip_address, + total_time, + ) return port_connection ################################################################################################ diff --git a/src/tm_devices/drivers/margin_testers/margin_tester.py b/src/tm_devices/drivers/margin_testers/margin_tester.py index 9cd84f6c..264e115f 100644 --- a/src/tm_devices/drivers/margin_testers/margin_tester.py +++ b/src/tm_devices/drivers/margin_testers/margin_tester.py @@ -148,12 +148,10 @@ def _open(self) -> bool: time.sleep(15) self._is_open = self.wait_for_network_connection( 120, - verbose=False, accept_immediate_connection=bool(os.environ.get("TM_DEVICES_UNIT_TESTS_RUNNING")), ) self._is_open &= self.wait_for_api_connection( 120, - verbose=False, accept_immediate_connection=True, ) return self._is_open diff --git a/src/tm_devices/drivers/scopes/tekscope/tekscope.py b/src/tm_devices/drivers/scopes/tekscope/tekscope.py index b3aeddaf..48b9119e 100644 --- a/src/tm_devices/drivers/scopes/tekscope/tekscope.py +++ b/src/tm_devices/drivers/scopes/tekscope/tekscope.py @@ -1,5 +1,7 @@ """Base TekScope scope device driver module.""" +import logging + # pylint: disable=too-many-lines import math import os @@ -58,6 +60,8 @@ SignalGeneratorOutputPathsBase, ) +_logger: logging.Logger = logging.getLogger(__name__) + @dataclass(frozen=True) class TekScopeSourceDeviceConstants(SourceDeviceConstants): @@ -376,6 +380,7 @@ def curve_query( # noqa: PLR0912,C901 break # break out of loop if not found: warnings.warn(f"source not available for curve query: CH{channel_num}", stacklevel=2) + _logger.warning("source not available for curve query: CH%d", channel_num) return [] self.set_and_check(":DATA:ENC", "ASCII") diff --git a/src/tm_devices/drivers/scopes/tekscope/tekscopepc.py b/src/tm_devices/drivers/scopes/tekscope/tekscopepc.py index 4d1554c7..d4b49811 100644 --- a/src/tm_devices/drivers/scopes/tekscope/tekscopepc.py +++ b/src/tm_devices/drivers/scopes/tekscope/tekscopepc.py @@ -1,5 +1,6 @@ """TekScopePC device driver module.""" +import logging import warnings from tm_devices.commands import TekScopePCMixin @@ -7,6 +8,8 @@ from tm_devices.drivers.scopes.tekscope.tekscope import AbstractTekScope from tm_devices.helpers import ReadOnlyCachedProperty as cached_property # noqa: N813 +_logger: logging.Logger = logging.getLogger(__name__) + @family_base_class class TekScopePC(TekScopePCMixin, AbstractTekScope): # pyright: ignore[reportIncompatibleVariableOverride] @@ -31,8 +34,9 @@ def total_channels(self) -> int: ################################################################################################ def _reboot(self) -> None: """Reboot the device.""" - warnings.warn( + msg = ( f"Rebooting is not supported for the {self.__class__.__name__} driver, " - f"{self._name_and_alias} will not be rebooted.", - stacklevel=3, + f"{self._name_and_alias} will not be rebooted." ) + _logger.warning(msg) + warnings.warn(msg, stacklevel=3) diff --git a/src/tm_devices/drivers/scopes/tekscope_2k/tekscope_2k.py b/src/tm_devices/drivers/scopes/tekscope_2k/tekscope_2k.py index 82863616..87f3a3b5 100644 --- a/src/tm_devices/drivers/scopes/tekscope_2k/tekscope_2k.py +++ b/src/tm_devices/drivers/scopes/tekscope_2k/tekscope_2k.py @@ -1,5 +1,6 @@ """Base TekScope2k scope device driver module.""" +import logging import warnings from abc import ABC @@ -16,6 +17,8 @@ from tm_devices.drivers.scopes.scope import Scope from tm_devices.helpers import ReadOnlyCachedProperty as cached_property # noqa: N813 +_logger: logging.Logger = logging.getLogger(__name__) + @family_base_class class TekScope2k(_TektronixPIScopeMixin, PIControl, Scope, ChannelControlMixin, ABC): @@ -97,6 +100,7 @@ def curve_query( # pylint: disable=too-many-locals break # break out of loop if not found: warnings.warn(f"source not available for curve query: CH{channel_num}", stacklevel=2) + _logger.warning("source not available for curve query: CH%d", channel_num) return [] self.set_and_check(":DATA:ENC", "ASCI") diff --git a/src/tm_devices/helpers/__init__.py b/src/tm_devices/helpers/__init__.py index de918cb5..97192226 100644 --- a/src/tm_devices/helpers/__init__.py +++ b/src/tm_devices/helpers/__init__.py @@ -30,11 +30,9 @@ create_visa_connection, detect_visa_resource_expression, get_model_series, - get_timestamp_string, get_version, get_visa_backend, ping_address, - print_with_timestamp, register_additional_usbtmc_mapping, sanitize_enum, ) @@ -57,12 +55,10 @@ "DeviceTypes", "DMConfigOptions", "get_model_series", - "get_timestamp_string", "get_version", "get_visa_backend", "PACKAGE_NAME", "ping_address", - "print_with_timestamp", "PYVISA_PY_BACKEND", "raise_error", "raise_failure", diff --git a/src/tm_devices/helpers/enums.py b/src/tm_devices/helpers/enums.py index c403b9b5..19102948 100644 --- a/src/tm_devices/helpers/enums.py +++ b/src/tm_devices/helpers/enums.py @@ -26,6 +26,23 @@ def list_values(cls) -> List[str]: return [enum_entry.value for enum_entry in cls] +class LoggingLevels(CustomStrEnum): + """A class holding the valid logging levels supported.""" + + DEBUG = "DEBUG" + """An enum member representing the DEBUG logging level.""" + INFO = "INFO" + """An enum member representing the INFO logging level.""" + WARNING = "WARNING" + """An enum member representing the WARNING logging level.""" + ERROR = "ERROR" + """An enum member representing the ERROR logging level.""" + CRITICAL = "CRITICAL" + """An enum member representing the CRITICAL logging level.""" + NONE = "NONE" + """An enum member indicating no logging messages should be captured.""" + + class ConfigFileType(CustomStrEnum): """Class holding valid config file extensions.""" diff --git a/src/tm_devices/helpers/functions.py b/src/tm_devices/helpers/functions.py index f3f9859e..f1550743 100644 --- a/src/tm_devices/helpers/functions.py +++ b/src/tm_devices/helpers/functions.py @@ -1,9 +1,9 @@ """Module containing helpers for the `tm_devices` package.""" import contextlib -import datetime import importlib.metadata import json +import logging import platform import re import shlex @@ -18,7 +18,6 @@ import requests -from dateutil.tz import tzlocal from packaging.version import InvalidVersion, Version from tm_devices.helpers.constants_and_dataclasses import ( @@ -34,6 +33,8 @@ from tm_devices.helpers.enums import CustomStrEnum, SupportedModels from tm_devices.helpers.standalone_functions import validate_address +_logger: logging.Logger = logging.getLogger(__name__) + with warnings.catch_warnings(): warnings.simplefilter("ignore", UserWarning) import pyvisa as visa @@ -186,54 +187,56 @@ def check_for_update(package_name: str = PACKAGE_NAME, index_name: str = "pypi") latest_version = version_list[0] if installed_version != latest_version: - print( - f"\n\n\033[91mVersion {latest_version} of " - f"{package_name} is available on {index_name}.org.\n" - f"Version {installed_version} of " - f"{package_name} is currently installed.\n\n" - f"To upgrade {package_name} run the following command: " - f"python -m pip install -U {package_name}\n\n\033[0m" + _logger.debug( + "\n\n\033[91mVersion %(latest_version)s of " + "%(package_name)s is available on %(index_name)s.org.\n" + "Version %(installed_version)s of " + "%(package_name)s is currently installed.\n\n" + "To upgrade %(package_name)s run the following command: " + "python -m pip install -U %(package_name)s\n\n\033[0m", + { + "latest_version": latest_version, + "package_name": package_name, + "index_name": index_name, + "installed_version": installed_version, + }, ) except ModuleNotFoundError: - print( - f"\n\n\033[91m{package_name} is not installed, " - f"unable to check for updates.\n\n\033[0m" + _logger.warning( + "\n\n\033[91m%s is not installed, unable to check for updates.\n\n\033[0m", + package_name, ) except (IndexError, ValueError): - print( - f"\n\n\033[91m{package_name} is not available on {index_name}.org, " - f"unable to check for updates.\n\n\033[0m" + _logger.warning( + "\n\n\033[91m%s is not available on %s.org, unable to check for updates.\n\n\033[0m", + package_name, + index_name, ) -def check_network_connection( - device_name: str, ip_address: str, verbose: bool = True -) -> Tuple[bool, str]: +def check_network_connection(device_name: str, ip_address: str) -> Tuple[bool, str]: """Check the network connection to the device using the external ping command. Args: device_name: The name of the device. ip_address: The ip address of the device. - verbose: Set this to False in order to disable printouts. Returns: A boolean indicating if there is a network connection and a string with the result of the ping command. """ - if verbose: - print_with_timestamp(f"({device_name}) ping >> {ip_address}") - print(f"{get_timestamp_string()} - ") + _logger.debug("(%s) ping >> %s", device_name, ip_address) ping_result = ping_address(ip_address) - if verbose: - print_with_timestamp("Response from ping >>") - for line in ping_result.strip().split("\n"): - print(f" {line}") + _logger.debug( + "Response from ping >>\n%s", + ping_result.strip(), + ) return "ttl=" in ping_result.lower(), ping_result def check_port_connection( - device_name: str, ip_address: str, port: int, timeout_seconds: int = 5, verbose: bool = True + device_name: str, ip_address: str, port: int, timeout_seconds: int = 5 ) -> bool: """Check if the given port is open on the device. @@ -242,13 +245,11 @@ def check_port_connection( ip_address: The ip address. port: The port to check. timeout_seconds: The number of seconds to use as the socket timeout. - verbose: Set this to False in order to disable printouts. Returns: A boolean indicating if the port is open. """ - if verbose: - print_with_timestamp(f"({device_name}) >> checking if port {port} is open on {ip_address}") + _logger.debug("(%s) >> checking if port %d is open on %s", device_name, port, ip_address) with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as temp_socket: try: @@ -259,13 +260,12 @@ def check_port_connection( except (socket.error, socket.gaierror, socket.herror): port_open = False - if verbose: - print_with_timestamp(f"Result >> {port_open}") + _logger.debug("(%s) port %d open = %s", device_name, port, port_open) return port_open def check_visa_connection( - config_entry: DeviceConfigEntry, visa_library: str, device_name: str, verbose: bool = True + config_entry: DeviceConfigEntry, visa_library: str, device_name: str ) -> bool: """Check if a VISA connection can be made to the device. @@ -273,16 +273,15 @@ def check_visa_connection( config_entry: The device listed in the config of the framework to check. visa_library: Indicates which visa library to use for the connection. device_name: The name of the device. - verbose: Set this to False in order to disable printouts. Returns: a boolean indicating if the visa connection was successful """ - if verbose: - print_with_timestamp( - f"({device_name}) >> checking if a VISA connection can be made to " - f"{config_entry.get_visa_resource_expression()}" - ) + _logger.debug( + "(%s) >> checking if a VISA connection can be made to %s", + device_name, + config_entry.get_visa_resource_expression(), + ) try: # Create new VISA connection temp_resource = create_visa_connection( @@ -294,8 +293,7 @@ def check_visa_connection( except ConnectionError: # raised by the create_visa_connection() function visa_connected = False - if verbose: - print_with_timestamp(f"Result >> {visa_connected}") + _logger.debug("(%s) VISA connected = %s", device_name, visa_connected) return visa_connected @@ -331,12 +329,15 @@ def create_visa_connection( if ( visa_object.visalib.library_path.path == "py" and visa_library != PYVISA_PY_BACKEND ): # pragma: no cover - warnings.warn( + warning_msg = ( "No VISA installation was detected, defaulting to using " "PyVISA-py as the VISA backend.\n\n" - 'Add the "STANDALONE" option to the configuration to silence this warning.', - stacklevel=4, + 'Add the "STANDALONE" option to the configuration or set the ' + "`DeviceManager.visa_library` attribute to a valid VISA library to " + "silence this warning." ) + warnings.warn(warning_msg, stacklevel=4) + _logger.warning(warning_msg) # The broad except is because pyvisa_py can throw a base exception in the tcpip.py file except Exception as error_1: if not retry_connection: @@ -466,15 +467,11 @@ def get_model_series(model: str) -> str: if not model_series: if model not in SupportedModels.list_values(): warnings.warn(f'The "{model}" model is not supported by {PACKAGE_NAME}', stacklevel=2) + _logger.warning('The "%s" model is not supported by %s', model, PACKAGE_NAME) model_series = model return model_series -def get_timestamp_string() -> str: - """Return a string containing the current timestamp.""" - return str(datetime.datetime.now(tz=tzlocal()))[:-3] - - def get_version(version_string: str) -> Version: """Get a Version object from a string. @@ -564,18 +561,6 @@ def ping_address(address: str, timeout: int = 2) -> str: return output.replace("\r\n", "\n") -def print_with_timestamp(message: str, end: str = "\n") -> str: - """Print and return a string prepended with a timestamp. - - Args: - message: The message to print. - end: The end of the line to print. - """ - message = f"{get_timestamp_string()} - {message}" - print(message, end=end) - return message - - def register_additional_usbtmc_mapping(model_series: str, *, model_id: str, vendor_id: str) -> None: """Register USBTMC connection information for a device that doesn't have native `tm_devices` USBTMC support. diff --git a/src/tm_devices/helpers/logging.py b/src/tm_devices/helpers/logging.py new file mode 100644 index 00000000..012ccfbd --- /dev/null +++ b/src/tm_devices/helpers/logging.py @@ -0,0 +1,83 @@ +"""Helpers for logging.""" + +import importlib.metadata +import logging +import sys +import time + +from pathlib import Path +from typing import Optional + +import colorlog + +from tzlocal import get_localzone # pyright: ignore[reportUnknownVariableType] + +from tm_devices.helpers.constants_and_dataclasses import PACKAGE_NAME +from tm_devices.helpers.enums import LoggingLevels + + +def configure_logging( + console_logging_level: LoggingLevels = LoggingLevels.INFO, + file_logging_level: LoggingLevels = LoggingLevels.DEBUG, + log_file_directory: Optional[Path] = None, + log_file_name: Optional[str] = None, +) -> logging.Logger: + """Configure the logging for this package. + + Args: + console_logging_level: The logging level to set for the console. Defaults to INFO. Set to + [`LoggingLevels.NONE`][tm_devices.helpers.enums.LoggingLevels.NONE] to disable all + console logging/printouts except for certain warnings and exceptions. + file_logging_level: The logging level to set for the file. Defaults to DEBUG. Set to + [`LoggingLevels.NONE`][tm_devices.helpers.enums.LoggingLevels.NONE] to disable logging + to a file entirely. + log_file_directory: The directory to save the log file to. Defaults to "./logs" in the + current working directory. + log_file_name: The name of the log file to save the logs to. Defaults to a timestamped name. + + Returns: + The base logger for the package, this base logger can also be accessed using + `logging.getLogger(tm_devices.PACKAGE_NAME)`. + """ + _logger: logging.Logger = logging.getLogger(PACKAGE_NAME) + # Set the logger level to the lowest level, the handlers will filter out specific levels + # based on user configuration + _logger.setLevel(logging.DEBUG) + _logger.addHandler(logging.NullHandler()) + # The logger/module name is not included in the message, since formatting the messages to + # be aligned would cause the width of the message prefix to be almost 100 characters before + # the message is even added to the line. + logging_file_format_string = "[%(asctime)s] [%(levelname)8s] %(message)s" + logging_console_format_string = "%(asctime)s - %(message)s" + if not log_file_directory: + log_file_directory = Path("logs") + if not log_file_name: + log_file_name = f"tm_devices_{time.strftime('%m-%d-%Y_%H-%M-%S', time.localtime())}.log" + log_filepath = log_file_directory / log_file_name + log_filepath.parent.mkdir(parents=True, exist_ok=True) + + if file_logging_level != LoggingLevels.NONE: + file_handler = logging.FileHandler(log_filepath, mode="w", encoding="utf-8") + file_handler.setLevel(getattr(logging, file_logging_level.value)) + file_formatter = logging.Formatter(logging_file_format_string) + file_formatter.default_msec_format = "%s.%06d" # Use 6 digits of precision for milliseconds + file_handler.setFormatter(file_formatter) + _logger.addHandler(file_handler) + + # Log a few things to just the file + _logger.debug("timezone==%s", get_localzone()) # pyright: ignore[reportUnknownArgumentType] + _logger.debug("%s==%s", PACKAGE_NAME, importlib.metadata.version(PACKAGE_NAME)) + + if console_logging_level != LoggingLevels.NONE: + console_handler = colorlog.StreamHandler(stream=sys.stdout) + console_handler.setLevel(getattr(logging, console_logging_level.value)) + console_formatter = colorlog.ColoredFormatter( + "%(log_color)s" + logging_console_format_string, log_colors=colorlog.default_log_colors + ) + console_formatter.default_msec_format = ( + "%s.%06d" # Use 6 digits of precision for milliseconds + ) + console_handler.setFormatter(console_formatter) + _logger.addHandler(console_handler) + + return _logger diff --git a/src/tm_devices/helpers/singleton_metaclass.py b/src/tm_devices/helpers/singleton_metaclass.py index aa2c224b..c5dad32e 100644 --- a/src/tm_devices/helpers/singleton_metaclass.py +++ b/src/tm_devices/helpers/singleton_metaclass.py @@ -1,10 +1,13 @@ """A Module containing a metaclass that converts any class into a Singleton.""" +import logging import warnings from typing import Any, MutableMapping from weakref import WeakValueDictionary +_logger: logging.Logger = logging.getLogger(__name__) + class Singleton(type): """A metaclass that converts a standard class into a Singleton (one instance). @@ -23,10 +26,11 @@ def __call__(cls, *args: Any, **kwargs: Any) -> Any: new_instance = super(Singleton, cls).__call__(*args, **kwargs) # noqa: UP008 cls._class_instances[cls] = new_instance else: - warnings.warn( + msg = ( f"The {cls.__name__} has already been created and is not " f"allowed to be instantiated twice. Previously created instance " - f"will be used instead.\n", - stacklevel=3, + f"will be used instead." ) + warnings.warn(msg, stacklevel=3) + _logger.warning(msg) return cls._class_instances[cls] diff --git a/src/tm_devices/helpers/verification_functions.py b/src/tm_devices/helpers/verification_functions.py index eaea9938..bcbe9f6f 100644 --- a/src/tm_devices/helpers/verification_functions.py +++ b/src/tm_devices/helpers/verification_functions.py @@ -4,8 +4,6 @@ from typing import Tuple, Union -from tm_devices.helpers.functions import get_timestamp_string - def raise_error(unique_identifier: str, message: str, *, condense_printout: bool = True) -> None: """Raise an AssertionError with the provided message indicating there was an error. @@ -18,12 +16,10 @@ def raise_error(unique_identifier: str, message: str, *, condense_printout: bool Raises: AssertionError: Prints out the error message with a traceback. """ - # TODO: integrate this with logging - # https://github.com/tektronix/tm_devices/issues/316 if condense_printout: # Make the message smaller message = ", ".join([x.strip() for x in message.split("\n")]) - message = f"{get_timestamp_string()} - ERROR: ({unique_identifier}) : {message}" + message = f"ERROR: ({unique_identifier}) : {message}" raise AssertionError(message) @@ -38,12 +34,10 @@ def raise_failure(unique_identifier: str, message: str, *, condense_printout: bo Raises: AssertionError: Prints out the failure message with a traceback. """ - # TODO: integrate this with logging - # https://github.com/tektronix/tm_devices/issues/316 if condense_printout: # Make the message smaller message = ", ".join([x.strip() for x in message.split("\n")]) - message = f"{get_timestamp_string()} - FAILURE: ({unique_identifier}) : {message}" + message = f"FAILURE: ({unique_identifier}) : {message}" raise AssertionError(message) diff --git a/tests/conftest.py b/tests/conftest.py index e6e04dd2..f8912fe9 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -11,7 +11,7 @@ import pyvisa.constants from mock_server import mocker_server, PORT -from tm_devices import DeviceManager +from tm_devices import configure_logging, DeviceManager, LoggingLevels from tm_devices.components import DMConfigParser from tm_devices.helpers import DMConfigOptions, validate_address @@ -58,9 +58,9 @@ def _auto_add_newline_to_test_start() -> ( # pyright: ignore [reportUnusedFunct Generator[None, None, None] ): """Automatically add a newline at the start of each test.""" - print(f"\n{'#' * 90}\nExecuting {os.environ['PYTEST_CURRENT_TEST'].split(' ')[0]}\n") + print(f"\n{'#' * 90}\nExecuting {os.environ['PYTEST_CURRENT_TEST'].split(' ')[0]}\n") # noqa: T201 yield - print(f"\n\nFinished {os.environ['PYTEST_CURRENT_TEST'].split(' ')[0]}\n{'#' * 90}") + print(f"\n\nFinished {os.environ['PYTEST_CURRENT_TEST'].split(' ')[0]}\n{'#' * 90}") # noqa: T201 @pytest.fixture(name="device_manager", scope="session") @@ -70,7 +70,8 @@ def fixture_device_manager() -> Generator[DeviceManager, None, None]: Yields: The DeviceManager instance. """ - print() + configure_logging(console_logging_level=LoggingLevels.DEBUG) + print() # noqa: T201 with mock.patch( "socket.gethostbyname", mock.MagicMock(side_effect=mock_gethostbyname) ), mock.patch( diff --git a/tests/test_all_device_drivers.py b/tests/test_all_device_drivers.py index e9819d9d..bacb1a98 100644 --- a/tests/test_all_device_drivers.py +++ b/tests/test_all_device_drivers.py @@ -205,7 +205,7 @@ def test_all_device_drivers() -> None: sorted_created_connections_list == supported_connections_list ), f"Some connections are not tested: {connections_without_testing=}" - print(f"\nVerified all {len(SIMULATED_DEVICE_LIST)} device drivers") - print( + print(f"\nVerified all {len(SIMULATED_DEVICE_LIST)} device drivers") # noqa: T201 + print( # noqa: T201 f"{len(drivers_with_auto_generated_commands)} device drivers have auto-generated commands" ) diff --git a/tests/test_config_parser.py b/tests/test_config_parser.py index 15f70357..a96f1589 100644 --- a/tests/test_config_parser.py +++ b/tests/test_config_parser.py @@ -64,7 +64,7 @@ def test_environment_variable_config(capsys: pytest.CaptureFixture[str]) -> None f"TM_OPTIONS=DEFAULT_VISA_TIMEOUT=10000,STANDALONE\n" f"TM_DEVICES=~~~{expected_device_string}~~~" ) - print(config) + print(config) # noqa: T201 assert capsys.readouterr().out.strip() == expected_entry_string # test smu with serial properties @@ -104,7 +104,7 @@ def test_environment_variable_config(capsys: pytest.CaptureFixture[str]) -> None f"TM_OPTIONS=DEFAULT_VISA_TIMEOUT=10000,STANDALONE\n" f"TM_DEVICES=~~~{expected_device_string}~~~" ) - print(config) + print(config) # noqa: T201 assert capsys.readouterr().out.strip() == expected_entry_string expected_device = DeviceConfigEntry( diff --git a/tests/test_device_manager.py b/tests/test_device_manager.py index 1eb1a480..38f35f00 100644 --- a/tests/test_device_manager.py +++ b/tests/test_device_manager.py @@ -103,7 +103,7 @@ def test_get_config_methods_with_devices(self, device_manager: DeviceManager) -> ) with open("./temp_config.yaml", encoding="utf-8") as temp_config: text = temp_config.read() - print(text) + print(text) # noqa: T201 assert ( text == f"""--- diff --git a/tests/test_helpers.py b/tests/test_helpers.py index c9862c23..850430af 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -1,7 +1,6 @@ # pyright: reportPrivateUsage=none """Tests for the helpers subpackage.""" -import datetime import random import socket @@ -12,11 +11,9 @@ from typing import Any, ClassVar, Dict, List, Optional, Tuple from unittest import mock -import dateutil.parser import pytest import pyvisa as visa -from dateutil.tz import tzlocal from packaging.version import InvalidVersion, Version from requests import Response @@ -35,7 +32,6 @@ get_version, get_visa_backend, ping_address, - print_with_timestamp, sanitize_enum, SupportedModels, VALID_DEVICE_CONNECTION_TYPES, @@ -138,7 +134,7 @@ def test_check_network_connection() -> None: stdout = StringIO() with redirect_stdout(stdout): - assert check_network_connection("name", "127.0.0.1", verbose=False)[0] + assert check_network_connection("name", "127.0.0.1")[0] assert stdout.getvalue() == "" @@ -157,35 +153,10 @@ def test_check_port_connection() -> None: with redirect_stdout(stdout), mock.patch( "socket.socket.connect", mock.MagicMock(side_effect=socket.error("")) ): - assert not check_port_connection( - "name", "127.0.0.1", 55555, timeout_seconds=1, verbose=False - ) + assert not check_port_connection("name", "127.0.0.1", 55555, timeout_seconds=1) assert stdout.getvalue() == "" -def test_print_with_timestamp() -> None: - """Test the print_with_timestamp helper function.""" - stdout = StringIO() - with redirect_stdout(stdout): - now = datetime.datetime.now(tz=tzlocal()) - print_with_timestamp("message") - - message = stdout.getvalue() - message_parts = message.split(" - ") - assert len(message_parts) == 2 - assert message_parts[1] == "message\n" - parsed_datetime = dateutil.parser.parse(message_parts[0].strip()) - allowed_difference = datetime.timedelta( - days=0, - hours=0, - minutes=0, - seconds=1, - microseconds=0, - ) - calculated_difference = abs(parsed_datetime - now) - assert calculated_difference <= allowed_difference - - def test_sanitizing_enums() -> None: """Verify that the sanitization functions work.""" assert sanitize_enum("SCOPE", DeviceTypes) == DeviceTypes.SCOPE @@ -260,15 +231,13 @@ def test_create_and_check_visa_connection(capsys: pytest.CaptureFixture[str]) -> assert "checking if a VISA connection can be made to " in stdout assert "Result >> True" in stdout - assert check_visa_connection(dev_config_3, SIMULATED_VISA_LIB, "dev_config_3", verbose=False) + assert check_visa_connection(dev_config_3, SIMULATED_VISA_LIB, "dev_config_3") stdout = capsys.readouterr().out assert "checking if a VISA connection can be made to " not in stdout assert "Result >> True" not in stdout with mock.patch("pyvisa.ResourceManager", mock.MagicMock(side_effect=visa.Error())): - assert not check_visa_connection( - dev_config_3, SIMULATED_VISA_LIB, "dev_config_3", verbose=False - ) + assert not check_visa_connection(dev_config_3, SIMULATED_VISA_LIB, "dev_config_3") def test_check_for_update(capsys: pytest.CaptureFixture[str]) -> None: diff --git a/tests/test_rest_api_device.py b/tests/test_rest_api_device.py index c609f130..babd41fb 100644 --- a/tests/test_rest_api_device.py +++ b/tests/test_rest_api_device.py @@ -182,7 +182,7 @@ def test_set_api_version_non_verbose(rest_api_control: CustomRestApiDevice) -> N rest_api_control: Rest API Device. """ rest_api_control.API_VERSIONS = MappingProxyType({1: "/api", 2: "/api2"}) - rest_api_control.set_api_version(api_version=2, verbose=False) + rest_api_control.set_api_version(api_version=2) assert rest_api_control.api_url == rest_api_control.base_url + rest_api_control.API_VERSIONS[2] diff --git a/tests/test_scopes.py b/tests/test_scopes.py index 0755ee54..9363d607 100644 --- a/tests/test_scopes.py +++ b/tests/test_scopes.py @@ -376,14 +376,14 @@ def test_tekscope70k(device_manager: DeviceManager, capsys: pytest.CaptureFixtur assert scope.hostname == "" # Test some generic device functionality assert scope.wait_for_network_connection( - wait_time=0.05, sleep_seconds=1, accept_immediate_connection=True, verbose=True + wait_time=0.05, sleep_seconds=1, accept_immediate_connection=True ) assert ( f"Successfully established a network connection with {scope.ip_address}" in capsys.readouterr().out ) assert scope.wait_for_network_connection( - wait_time=0.05, sleep_seconds=1, accept_immediate_connection=True, verbose=False + wait_time=0.05, sleep_seconds=1, accept_immediate_connection=True ) assert ( f"Successfully established a network connection with {scope.ip_address}" @@ -393,7 +393,7 @@ def test_tekscope70k(device_manager: DeviceManager, capsys: pytest.CaptureFixtur "subprocess.check_output", mock.MagicMock(side_effect=subprocess.CalledProcessError(1, "")) ): assert not scope.wait_for_network_connection( - wait_time=0.05, sleep_seconds=1, accept_immediate_connection=True, verbose=True + wait_time=0.05, sleep_seconds=1, accept_immediate_connection=True ) assert ( f"Unable to establish a network connection with {scope.ip_address}" @@ -401,7 +401,7 @@ def test_tekscope70k(device_manager: DeviceManager, capsys: pytest.CaptureFixtur ) with pytest.raises(AssertionError): scope.wait_for_network_connection( - wait_time=0.05, sleep_seconds=1, accept_immediate_connection=False, verbose=False + wait_time=0.05, sleep_seconds=1, accept_immediate_connection=False ) scope.expect_esr(0) with pytest.raises(SystemError): diff --git a/tests/test_singleton.py b/tests/test_singleton.py index 701c2ff5..eeaf8719 100644 --- a/tests/test_singleton.py +++ b/tests/test_singleton.py @@ -13,7 +13,7 @@ class DummyClass(metaclass=Singleton): def __init__(self, value: bool = False) -> None: """Create an instance of the dummy class.""" - print("running init") + print("running init") # noqa: T201 self.init_count += 1 self.value = value diff --git a/tests/test_smu.py b/tests/test_smu.py index 2d02fd6f..d1c72800 100644 --- a/tests/test_smu.py +++ b/tests/test_smu.py @@ -138,7 +138,7 @@ def test_smu( # noqa: PLR0915 f" object at {id(smu)} address='SMU2601B-HOSTNAME' @@ -183,14 +183,14 @@ def test_smu( # noqa: PLR0915 "socket.socket.shutdown", mock.MagicMock(return_value=None) ): assert smu.wait_for_port_connection( - 4000, wait_time=0.05, sleep_seconds=0, accept_immediate_connection=True, verbose=True + 4000, wait_time=0.05, sleep_seconds=0, accept_immediate_connection=True ) assert ( f"Successfully established a connection to port 4000 on {smu.ip_address}" in capsys.readouterr().out ) assert smu.wait_for_port_connection( - 4000, wait_time=0.05, sleep_seconds=0, accept_immediate_connection=True, verbose=False + 4000, wait_time=0.05, sleep_seconds=0, accept_immediate_connection=True ) assert ( f"Successfully established a connection to port 4000 on {smu.ip_address}" @@ -202,11 +202,10 @@ def test_smu( # noqa: PLR0915 wait_time=0.05, sleep_seconds=0, accept_immediate_connection=False, - verbose=False, ) with mock.patch("socket.socket.connect", mock.MagicMock(side_effect=socket.error(""))): assert not smu.wait_for_port_connection( - 4000, wait_time=0.05, sleep_seconds=0, accept_immediate_connection=True, verbose=True + 4000, wait_time=0.05, sleep_seconds=0, accept_immediate_connection=True ) assert ( f"Successfully established a connection to port 4000 on {smu.ip_address}" diff --git a/tests/validate_device_manager_delete.py b/tests/validate_device_manager_delete.py index af27b597..9c03c7b9 100644 --- a/tests/validate_device_manager_delete.py +++ b/tests/validate_device_manager_delete.py @@ -11,9 +11,11 @@ import pyvisa.constants from conftest import mock_gethostbyaddr, mock_gethostbyname, SIMULATED_VISA_LIB -from tm_devices import DeviceManager, PYVISA_PY_BACKEND +from tm_devices import configure_logging, DeviceManager, LoggingLevels, PYVISA_PY_BACKEND from tm_devices.helpers import DMConfigOptions +configure_logging(console_logging_level=LoggingLevels.DEBUG) + @mock.patch("socket.gethostbyname", mock.MagicMock(side_effect=mock_gethostbyname)) @mock.patch("socket.gethostbyaddr", mock.MagicMock(side_effect=mock_gethostbyaddr)) @@ -48,7 +50,7 @@ def verify_deleting_device_manager() -> None: del dev_manager stdout = stdout_buffer.getvalue() - print(stdout) + print(stdout) # noqa: T201 assert "Closing Connections to Devices" in stdout assert "Closing Connection to AFG 1" in stdout assert "DeviceManager Closed" in stdout @@ -67,7 +69,7 @@ def verify_deleting_device_manager() -> None: del dev_manager stdout = stdout_buffer.getvalue() - print(stdout) + print(stdout) # noqa: T201 assert stdout == "" # Test that the closing happens when the python interpreter exits diff --git a/tests/verify_physical_device_support.py b/tests/verify_physical_device_support.py index 57b1c9eb..85209727 100644 --- a/tests/verify_physical_device_support.py +++ b/tests/verify_physical_device_support.py @@ -18,6 +18,6 @@ device_manager.setup_cleanup_enabled = False device_manager.teardown_cleanup_enabled = False for device in device_manager.devices.values(): - print(device) + print(device) # noqa: T201 device.cleanup() assert not device.has_errors() From 626758814b46c8b931025a1251e5606bc5cd11a6 Mon Sep 17 00:00:00 2001 From: "Felt, Nicholas" Date: Fri, 8 Nov 2024 16:23:06 -0800 Subject: [PATCH 03/11] feat: Added the pyvisa log to the file. Also updated the tests to work properly with the new logging --- CHANGELOG.md | 3 + pyproject.toml | 1 + src/tm_devices/device_manager.py | 14 ++--- .../_abstract_device_control.py | 4 +- .../device_control/pi_control.py | 12 ++-- .../device_control/rest_api_control.py | 27 ++------ .../device_control/tsp_control.py | 17 +++-- src/tm_devices/drivers/device.py | 4 +- .../drivers/margin_testers/margin_tester.py | 4 +- src/tm_devices/drivers/margin_testers/tmt4.py | 20 +++--- .../drivers/scopes/tekscope/mso2.py | 4 +- .../drivers/scopes/tekscope/tekscope.py | 4 +- .../tekscope_5k_7k_70k/tekscope_5k_7k_70k.py | 4 +- src/tm_devices/drivers/scopes/tso/tsovu.py | 4 +- src/tm_devices/helpers/__init__.py | 2 + src/tm_devices/helpers/functions.py | 2 +- src/tm_devices/helpers/logging.py | 62 +++++++++++++++---- tests/conftest.py | 26 +++++++- tests/test_helpers.py | 22 ++----- tests/test_pi_device.py | 26 +++----- tests/test_rest_api_device.py | 2 +- tests/test_scopes.py | 36 ++++++----- tests/test_smu.py | 25 +++++--- tests/test_verification_functions.py | 6 +- 24 files changed, 201 insertions(+), 130 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c1c60602..6fdbd033 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,9 @@ However, please read through all changes to be aware of what may potentially imp - _**BREAKING CHANGE**_: Changed the behavior of the `expect_esr()` method to expect an integer error code input and an optional tuple of error messages to compare against the actual error code and messages returned by the `_get_errors()` private method. - _**minor breaking change**_: Converted the `device_type` property into an abstract, cached property to force all children of the `Device` class to specify what type of device they are. - Updated the auto-generated command mixin classes to no longer use an `__init__()` method to enable the driver API documentation to render in a more usable way. +- Switched from using standard `print()` calls to using the `logging` module for all logging in the `tm_devices` package. + - A configuration function provides the ability to set different logging levels for stdout and file logging. + - The debug logging from the `pyvisa` package is also included in the log file by default. ### Removed diff --git a/pyproject.toml b/pyproject.toml index ecdb75f1..f4cfaaf6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -290,6 +290,7 @@ filterwarnings = [ ] junit_family = "xunit2" junit_logging = "all" +log_format = "[%(asctime)s] [%(levelname)8s] %(message)s" markers = [ 'docs', 'order', diff --git a/src/tm_devices/device_manager.py b/src/tm_devices/device_manager.py index 8d0cbc6d..6696cd28 100644 --- a/src/tm_devices/device_manager.py +++ b/src/tm_devices/device_manager.py @@ -37,6 +37,7 @@ from tm_devices.helpers import ( AliasDict, check_for_update, + configure_logging, ConnectionTypes, create_visa_connection, detect_visa_resource_expression, @@ -135,11 +136,7 @@ def __init__( (updates the current configuration options). external_device_drivers: An optional dict for passing in additional device drivers. """ - # pylint: disable=import-outside-toplevel - if not logging.getLogger(PACKAGE_NAME).hasHandlers(): - from tm_devices.helpers.logging import configure_logging - - configure_logging() + configure_logging() self._suppress_protection = False # Set up the DeviceManager self.__is_open = False @@ -260,8 +257,11 @@ def verbose_visa(self, value: bool) -> None: visa.log_to_screen() else: for handler in visa.logger.handlers.copy(): - if "DEBUG" in str(handler): - visa.logger.removeHandler(handler) + if "DEBUG" in str(handler) and ( + isinstance(handler, logging.StreamHandler) + and not isinstance(handler, logging.FileHandler) + ): + visa.logger.removeHandler(handler) # pyright: ignore[reportUnknownArgumentType] self.__config.options.verbose_visa = value self.__verbose_visa = value diff --git a/src/tm_devices/driver_mixins/device_control/_abstract_device_control.py b/src/tm_devices/driver_mixins/device_control/_abstract_device_control.py index 3756411f..e6d19297 100644 --- a/src/tm_devices/driver_mixins/device_control/_abstract_device_control.py +++ b/src/tm_devices/driver_mixins/device_control/_abstract_device_control.py @@ -15,7 +15,9 @@ def __init__(self, config_entry: DeviceConfigEntry, verbose: bool) -> None: Args: config_entry: A config entry object parsed by the DMConfigParser. - verbose: A boolean indicating if verbose output should be printed. + verbose: A boolean indicating if verbose output should be printed. If True, + communication printouts will be logged with a level of INFO. If False, + communication printouts will be logged with a level of DEBUG. """ self._is_open = False self._verbose = verbose diff --git a/src/tm_devices/driver_mixins/device_control/pi_control.py b/src/tm_devices/driver_mixins/device_control/pi_control.py index 0c1793d3..1e5f8b97 100644 --- a/src/tm_devices/driver_mixins/device_control/pi_control.py +++ b/src/tm_devices/driver_mixins/device_control/pi_control.py @@ -66,7 +66,9 @@ def __init__( Args: config_entry: A config entry object parsed by the DMConfigParser. - verbose: A boolean indicating if verbose output should be printed. + verbose: A boolean indicating if verbose output should be printed. If True, + communication printouts will be logged with a level of INFO. If False, + communication printouts will be logged with a level of DEBUG. visa_resource: The VISA resource object. default_visa_timeout: The default VISA timeout value in milliseconds. """ @@ -124,7 +126,7 @@ def visa_timeout(self, timeout_ms: float) -> None: timeout_ms: The new VISA timeout, in milliseconds. """ self._visa_resource.timeout = timeout_ms - _logger.debug("%s VISA timeout set to: %fms", self._name_and_alias, timeout_ms) + _logger.debug("%s VISA timeout set to: %.3fms", self._name_and_alias, timeout_ms) @property def visa_resource(self) -> visa.resources.MessageBasedResource: @@ -771,7 +773,6 @@ def wait_for_visa_connection( wait_time: float, sleep_seconds: int = 5, accept_immediate_connection: bool = False, - verbose: bool = True, ) -> bool: """Wait for a VISA connection to be made to the device. @@ -780,7 +781,6 @@ def wait_for_visa_connection( sleep_seconds: The number of seconds to sleep in between connection attempts. accept_immediate_connection: A boolean indicating if a connection on the first attempt is a valid connection. - verbose: Set this to False in order to disable printouts. Returns: A boolean indicating if a VISA connection was made within the given time limit. @@ -791,7 +791,7 @@ def wait_for_visa_connection( attempt_num = 0 visa_connection = False _logger.log( - logging.INFO if verbose else logging.DEBUG, + logging.INFO if self._verbose else logging.DEBUG, "Attempting to establish a VISA connection with %s", self._resource_expression, ) @@ -815,7 +815,7 @@ def wait_for_visa_connection( if visa_connection: _logger.log( - logging.INFO if verbose else logging.DEBUG, + logging.INFO if self._verbose else logging.DEBUG, "Successfully established a VISA connection with %s after %.2f seconds", self._resource_expression, total_time, diff --git a/src/tm_devices/driver_mixins/device_control/rest_api_control.py b/src/tm_devices/driver_mixins/device_control/rest_api_control.py index 4c8fe3d2..227639dd 100644 --- a/src/tm_devices/driver_mixins/device_control/rest_api_control.py +++ b/src/tm_devices/driver_mixins/device_control/rest_api_control.py @@ -43,7 +43,9 @@ def __init__(self, config_entry: DeviceConfigEntry, verbose: bool) -> None: Args: config_entry: A config entry object parsed by the DMConfigParser. - verbose: A boolean indicating if verbose output should be printed. + verbose: A boolean indicating if verbose output should be printed. If True, + communication printouts will be logged with a level of INFO. If False, + communication printouts will be logged with a level of DEBUG. """ super().__init__(config_entry, verbose) @@ -78,7 +80,6 @@ def delete( # noqa: PLR0913 allow_errors: bool = False, verify_ssl: bool = True, allow_redirects: bool = False, - verbose: bool = True, ) -> Tuple[bool, Union[Dict[str, Any], bytes], int, Optional[requests.RequestException]]: """Perform a DELETE request with the given url and headers. @@ -92,7 +93,6 @@ def delete( # noqa: PLR0913 verify_ssl: A bool that indicates if the SSL certificate should be verified. allow_redirects: A bool that indicates if URL redirects should be allowed. allow_errors: A boolean indicating if errors are allowed. - verbose: Set this to False in order to disable printouts. Returns: A tuple containing a success boolean, the response data, status code, and any errors @@ -107,7 +107,6 @@ def delete( # noqa: PLR0913 allow_errors=allow_errors, verify_ssl=verify_ssl, allow_redirects=allow_redirects, - verbose=verbose, ) # pylint: disable=too-many-arguments @@ -121,7 +120,6 @@ def get( # noqa: PLR0913 allow_errors: bool = False, verify_ssl: bool = True, allow_redirects: bool = False, - verbose: bool = True, ) -> Tuple[bool, Union[Dict[str, Any], bytes], int, Optional[requests.RequestException]]: """Perform a GET request with the given url and headers. @@ -135,7 +133,6 @@ def get( # noqa: PLR0913 verify_ssl: A bool that indicates if the SSL certificate should be verified. allow_redirects: A bool that indicates if URL redirects should be allowed. allow_errors: A boolean indicating if errors are allowed. - verbose: Set this to False in order to disable printouts. Returns: A tuple containing a success boolean, the response data, status code, and any errors @@ -150,7 +147,6 @@ def get( # noqa: PLR0913 allow_errors=allow_errors, verify_ssl=verify_ssl, allow_redirects=allow_redirects, - verbose=verbose, ) # pylint: disable=too-many-arguments @@ -166,7 +162,6 @@ def patch( # noqa: PLR0913 allow_errors: bool = False, verify_ssl: bool = True, allow_redirects: bool = False, - verbose: bool = True, ) -> Tuple[bool, Union[Dict[str, Any], bytes], int, Optional[requests.RequestException]]: """Perform a PATCH request with the given url and headers. @@ -185,7 +180,6 @@ def patch( # noqa: PLR0913 verify_ssl: A bool that indicates if the SSL certificate should be verified. allow_redirects: A bool that indicates if URL redirects should be allowed. allow_errors: A boolean indicating if errors are allowed. - verbose: Set this to False in order to disable printouts. Returns: A tuple containing a success boolean, the response data, status code, and any errors @@ -202,7 +196,6 @@ def patch( # noqa: PLR0913 allow_errors=allow_errors, verify_ssl=verify_ssl, allow_redirects=allow_redirects, - verbose=verbose, ) # pylint: disable=too-many-arguments @@ -218,7 +211,6 @@ def post( # noqa: PLR0913 allow_errors: bool = False, verify_ssl: bool = True, allow_redirects: bool = False, - verbose: bool = True, ) -> Tuple[bool, Union[Dict[str, Any], bytes], int, Optional[requests.RequestException]]: """Perform a POST request with the given url and headers. @@ -237,7 +229,6 @@ def post( # noqa: PLR0913 verify_ssl: A bool that indicates if the SSL certificate should be verified. allow_redirects: A bool that indicates if URL redirects should be allowed. allow_errors: A boolean indicating if errors are allowed. - verbose: Set this to False in order to disable printouts. Returns: A tuple containing a success boolean, the response data, status code, and any errors @@ -254,7 +245,6 @@ def post( # noqa: PLR0913 allow_errors=allow_errors, verify_ssl=verify_ssl, allow_redirects=allow_redirects, - verbose=verbose, ) # pylint: disable=too-many-arguments @@ -270,7 +260,6 @@ def put( # noqa: PLR0913 allow_errors: bool = False, verify_ssl: bool = True, allow_redirects: bool = False, - verbose: bool = True, ) -> Tuple[bool, Union[Dict[str, Any], bytes], int, Optional[requests.RequestException]]: """Perform a PUT request with the given url and headers. @@ -289,7 +278,6 @@ def put( # noqa: PLR0913 verify_ssl: A bool that indicates if the SSL certificate should be verified. allow_redirects: A bool that indicates if URL redirects should be allowed. allow_errors: A boolean indicating if errors are allowed. - verbose: Set this to False in order to disable printouts. Returns: A tuple containing a success boolean, the response data, status code, and any errors @@ -306,7 +294,6 @@ def put( # noqa: PLR0913 allow_errors=allow_errors, verify_ssl=verify_ssl, allow_redirects=allow_redirects, - verbose=verbose, ) def set_api_version(self, api_version: int) -> None: @@ -411,7 +398,6 @@ def _send_request( # noqa: PLR0913,PLR0912,C901 allow_errors: bool = False, verify_ssl: bool = True, allow_redirects: bool = False, - verbose: bool = True, ) -> Tuple[bool, Union[Dict[str, Any], bytes], int, Optional[requests.RequestException]]: """Perform a request with the given url and headers. @@ -431,7 +417,6 @@ def _send_request( # noqa: PLR0913,PLR0912,C901 verify_ssl: A bool that indicates if the SSL certificate should be verified. allow_redirects: A bool that indicates if URL redirects should be allowed. allow_errors: A boolean indicating if errors are allowed. - verbose: Set this to False in order to disable printouts. Returns: A tuple containing a success boolean, the response data, status code, and any errors @@ -450,10 +435,10 @@ def _send_request( # noqa: PLR0913,PLR0912,C901 response = cast(requests.Response, None) retval: Union[Dict[str, Any], bytes] = {} _logger.log( - logging.INFO if verbose else logging.DEBUG, + logging.INFO if self._verbose else logging.DEBUG, "(%s) %s >> %s%s%s", self._name_and_alias, - request_type.value, + getattr(request_type, "value", request_type), url, f", {headers=}" if headers else "", f", {json_body=}" if json_body else "", @@ -515,7 +500,7 @@ def _send_request( # noqa: PLR0913,PLR0912,C901 raise ValueError(msg) _logger.log( - logging.INFO if verbose else logging.DEBUG, + logging.INFO if self._verbose else logging.DEBUG, "Response from %s >>\n%s", request_type.value, json.dumps(response.json(), indent=2) if not return_bytes else response.text, diff --git a/src/tm_devices/driver_mixins/device_control/tsp_control.py b/src/tm_devices/driver_mixins/device_control/tsp_control.py index e632f836..a0253dab 100644 --- a/src/tm_devices/driver_mixins/device_control/tsp_control.py +++ b/src/tm_devices/driver_mixins/device_control/tsp_control.py @@ -169,11 +169,18 @@ def print_buffers(self, *args: str) -> None: def fix_width(key: str, value: Any) -> str: # Function to add spaces if needed return str(value) + " " * (column_widths[key] - len(str(value))) - print(*[fix_width(x, x) for x in buffer_data]) # noqa: T201 - for index in range(column_length): - print( # noqa: T201 - *[fix_width(k, v[index] if index < len(v) else "") for k, v in buffer_data.items()] - ) + buffer_headers = [fix_width(x, x) for x in buffer_data] + buffer_rows: List[List[Any]] = [ + [fix_width(k, v[index] if index < len(v) else "") for k, v in buffer_data.items()] + for index in range(column_length) + ] + _logger.info( + "(%s) Printing Buffers %s >>\n%s\n%s", + self._name_and_alias, + buffer_headers, + " ".join(buffer_headers), + "\n".join(" ".join(row) for row in buffer_rows), + ) def run_script(self, script_name: str) -> None: """Run a TSP script on the instrument. diff --git a/src/tm_devices/drivers/device.py b/src/tm_devices/drivers/device.py index f1156e0f..562222d8 100644 --- a/src/tm_devices/drivers/device.py +++ b/src/tm_devices/drivers/device.py @@ -73,7 +73,9 @@ def __init__(self, config_entry: DeviceConfigEntry, verbose: bool) -> None: Args: config_entry: A config entry object parsed by the DMConfigParser. - verbose: A boolean indicating if verbose output should be printed. + verbose: A boolean indicating if verbose output should be printed. If True, + communication printouts will be logged with a level of INFO. If False, + communication printouts will be logged with a level of DEBUG. """ super().__init__(config_entry, verbose) self._is_open = True diff --git a/src/tm_devices/drivers/margin_testers/margin_tester.py b/src/tm_devices/drivers/margin_testers/margin_tester.py index 264e115f..27161f45 100644 --- a/src/tm_devices/drivers/margin_testers/margin_tester.py +++ b/src/tm_devices/drivers/margin_testers/margin_tester.py @@ -27,7 +27,9 @@ def __init__(self, config_entry: DeviceConfigEntry, verbose: bool) -> None: Args: config_entry: A config entry object parsed by the DMConfigParser. - verbose: A boolean indicating if verbose output should be printed. + verbose: A boolean indicating if verbose output should be printed. If True, + communication printouts will be logged with a level of INFO. If False, + communication printouts will be logged with a level of DEBUG. """ super().__init__(config_entry, verbose) diff --git a/src/tm_devices/drivers/margin_testers/tmt4.py b/src/tm_devices/drivers/margin_testers/tmt4.py index 719e4e46..bfb687ca 100644 --- a/src/tm_devices/drivers/margin_testers/tmt4.py +++ b/src/tm_devices/drivers/margin_testers/tmt4.py @@ -26,7 +26,9 @@ def __init__(self, config_entry: DeviceConfigEntry, verbose: bool) -> None: Args: config_entry: A config entry object parsed by the DMConfigParser. - verbose: A boolean indicating if verbose output should be printed. + verbose: A boolean indicating if verbose output should be printed. If True, + communication printouts will be logged with a level of INFO. If False, + communication printouts will be logged with a level of DEBUG. Notes: The default port is 5000, but device could be configured for any specific port. @@ -110,11 +112,12 @@ def request_about_info(self, verbose: bool = True) -> Dict[str, Any]: A Dictionary with information about the Margin device. """ # Make get request to about and version endpoints - _, res_json, _, _ = self.get("/device/about", verbose=verbose) - res_json = cast(Dict[str, Any], res_json) - _, res_json2, _, _ = self.get("/device/version", verbose=verbose) - res_json2 = cast(Dict[str, Any], res_json2) - res_json.update(res_json2) + with self.temporary_verbose(verbose): + _, res_json, _, _ = self.get("/device/about") + res_json = cast(Dict[str, Any], res_json) + _, res_json2, _, _ = self.get("/device/version") + res_json2 = cast(Dict[str, Any], res_json2) + res_json.update(res_json2) return res_json def wait_till_unlocked(self, timeout: int = 120) -> None: @@ -144,8 +147,9 @@ def _check_api_connection(self) -> bool: Returns: A boolean indicating if the call was successful. """ - # allow_errors is set to True since this is just checking if the API is reachable - return self.get("/device/about", allow_errors=True, verbose=False)[0] + with self.temporary_verbose(False): + # allow_errors is set to True since this is just checking if the API is reachable + return self.get("/device/about", allow_errors=True)[0] def _cleanup(self) -> None: """Perform the cleanup defined for the device.""" diff --git a/src/tm_devices/drivers/scopes/tekscope/mso2.py b/src/tm_devices/drivers/scopes/tekscope/mso2.py index adfe019d..cc108dc6 100644 --- a/src/tm_devices/drivers/scopes/tekscope/mso2.py +++ b/src/tm_devices/drivers/scopes/tekscope/mso2.py @@ -27,7 +27,9 @@ def __init__( Args: config_entry: A config entry object parsed by the DMConfigParser. - verbose: A boolean indicating if verbose output should be printed. + verbose: A boolean indicating if verbose output should be printed. If True, + communication printouts will be logged with a level of INFO. If False, + communication printouts will be logged with a level of DEBUG. visa_resource: The VISA resource object. default_visa_timeout: The default VISA timeout value in milliseconds. """ diff --git a/src/tm_devices/drivers/scopes/tekscope/tekscope.py b/src/tm_devices/drivers/scopes/tekscope/tekscope.py index 48b9119e..a618f39b 100644 --- a/src/tm_devices/drivers/scopes/tekscope/tekscope.py +++ b/src/tm_devices/drivers/scopes/tekscope/tekscope.py @@ -125,7 +125,9 @@ def __init__( Args: config_entry: A config entry object parsed by the DMConfigParser. - verbose: A boolean indicating if verbose output should be printed. + verbose: A boolean indicating if verbose output should be printed. If True, + communication printouts will be logged with a level of INFO. If False, + communication printouts will be logged with a level of DEBUG. visa_resource: The VISA resource object. default_visa_timeout: The default VISA timeout value in milliseconds. """ diff --git a/src/tm_devices/drivers/scopes/tekscope_5k_7k_70k/tekscope_5k_7k_70k.py b/src/tm_devices/drivers/scopes/tekscope_5k_7k_70k/tekscope_5k_7k_70k.py index c9c47264..3d0a0289 100644 --- a/src/tm_devices/drivers/scopes/tekscope_5k_7k_70k/tekscope_5k_7k_70k.py +++ b/src/tm_devices/drivers/scopes/tekscope_5k_7k_70k/tekscope_5k_7k_70k.py @@ -32,7 +32,9 @@ def __init__( Args: config_entry: A config entry object parsed by the DMConfigParser. - verbose: A boolean indicating if verbose output should be printed. + verbose: A boolean indicating if verbose output should be printed. If True, + communication printouts will be logged with a level of INFO. If False, + communication printouts will be logged with a level of DEBUG. visa_resource: The VISA resource object. default_visa_timeout: The default VISA timeout value in milliseconds. """ diff --git a/src/tm_devices/drivers/scopes/tso/tsovu.py b/src/tm_devices/drivers/scopes/tso/tsovu.py index 01653ffe..0559c6ec 100644 --- a/src/tm_devices/drivers/scopes/tso/tsovu.py +++ b/src/tm_devices/drivers/scopes/tso/tsovu.py @@ -30,7 +30,9 @@ def __init__( Args: config_entry: A config entry object parsed by the DMConfigParser. - verbose: A boolean indicating if verbose output should be printed. + verbose: A boolean indicating if verbose output should be printed. If True, + communication printouts will be logged with a level of INFO. If False, + communication printouts will be logged with a level of DEBUG. visa_resource: The VISA resource object. default_visa_timeout: The default VISA timeout value in milliseconds. """ diff --git a/src/tm_devices/helpers/__init__.py b/src/tm_devices/helpers/__init__.py index 97192226..dc64d8de 100644 --- a/src/tm_devices/helpers/__init__.py +++ b/src/tm_devices/helpers/__init__.py @@ -36,6 +36,7 @@ register_additional_usbtmc_mapping, sanitize_enum, ) +from tm_devices.helpers.logging import configure_logging from tm_devices.helpers.read_only_cached_property import ReadOnlyCachedProperty from tm_devices.helpers.singleton_metaclass import Singleton from tm_devices.helpers.standalone_functions import validate_address @@ -49,6 +50,7 @@ "check_network_connection", "check_port_connection", "check_visa_connection", + "configure_logging", "create_visa_connection", "detect_visa_resource_expression", "DeviceConfigEntry", diff --git a/src/tm_devices/helpers/functions.py b/src/tm_devices/helpers/functions.py index f1550743..93803c74 100644 --- a/src/tm_devices/helpers/functions.py +++ b/src/tm_devices/helpers/functions.py @@ -229,7 +229,7 @@ def check_network_connection(device_name: str, ip_address: str) -> Tuple[bool, s ping_result = ping_address(ip_address) _logger.debug( "Response from ping >>\n%s", - ping_result.strip(), + "\n".join([" " + x for x in ping_result.strip().split("\n")]), ) return "ttl=" in ping_result.lower(), ping_result diff --git a/src/tm_devices/helpers/logging.py b/src/tm_devices/helpers/logging.py index 012ccfbd..fdfe53c4 100644 --- a/src/tm_devices/helpers/logging.py +++ b/src/tm_devices/helpers/logging.py @@ -9,19 +9,33 @@ from typing import Optional import colorlog +import pyvisa from tzlocal import get_localzone # pyright: ignore[reportUnknownVariableType] from tm_devices.helpers.constants_and_dataclasses import PACKAGE_NAME from tm_devices.helpers.enums import LoggingLevels +_logger_initialized = False + + +class _CustomFormatter(logging.Formatter): # pragma: no cover + def format(self, record: logging.LogRecord) -> str: + # Add the package name to the log record + record.package_name = record.name.split(".", maxsplit=1)[0] + # Call the original format method + return super().format(record) + def configure_logging( + *, console_logging_level: LoggingLevels = LoggingLevels.INFO, file_logging_level: LoggingLevels = LoggingLevels.DEBUG, log_file_directory: Optional[Path] = None, log_file_name: Optional[str] = None, -) -> logging.Logger: + colored_output: bool = True, + include_pyvisa_logs: bool = True, +) -> logging.Logger: # pragma: no cover """Configure the logging for this package. Args: @@ -34,12 +48,21 @@ def configure_logging( log_file_directory: The directory to save the log file to. Defaults to "./logs" in the current working directory. log_file_name: The name of the log file to save the logs to. Defaults to a timestamped name. + colored_output: Whether to use colored output from the `colorlog` package for the console. + Defaults to True. + include_pyvisa_logs: Whether to include logs from the `pyvisa` package in the log file. The + logging level will match the `file_logging_level`. Defaults to True. Returns: The base logger for the package, this base logger can also be accessed using `logging.getLogger(tm_devices.PACKAGE_NAME)`. """ + global _logger_initialized # noqa: PLW0603 + _logger: logging.Logger = logging.getLogger(PACKAGE_NAME) + if _logger_initialized: + # If the logger was previously initialized, just return it + return _logger # Set the logger level to the lowest level, the handlers will filter out specific levels # based on user configuration _logger.setLevel(logging.DEBUG) @@ -47,37 +70,52 @@ def configure_logging( # The logger/module name is not included in the message, since formatting the messages to # be aligned would cause the width of the message prefix to be almost 100 characters before # the message is even added to the line. - logging_file_format_string = "[%(asctime)s] [%(levelname)8s] %(message)s" + logging_file_format_string = "[%(asctime)s] [%(package_name)10s] [%(levelname)8s] %(message)s" logging_console_format_string = "%(asctime)s - %(message)s" if not log_file_directory: - log_file_directory = Path("logs") + log_file_directory = Path("./logs") if not log_file_name: - log_file_name = f"tm_devices_{time.strftime('%m-%d-%Y_%H-%M-%S', time.localtime())}.log" + log_file_name = f"{PACKAGE_NAME}_{time.strftime('%m-%d-%Y_%H-%M-%S', time.localtime())}.log" log_filepath = log_file_directory / log_file_name - log_filepath.parent.mkdir(parents=True, exist_ok=True) + # TODO: logging: look into rotating log files if file_logging_level != LoggingLevels.NONE: + # Set up logger for tm_devices + log_filepath.parent.mkdir(parents=True, exist_ok=True) file_handler = logging.FileHandler(log_filepath, mode="w", encoding="utf-8") - file_handler.setLevel(getattr(logging, file_logging_level.value)) - file_formatter = logging.Formatter(logging_file_format_string) + file_formatter = _CustomFormatter(logging_file_format_string) file_formatter.default_msec_format = "%s.%06d" # Use 6 digits of precision for milliseconds + + file_handler.setLevel(getattr(logging, file_logging_level.value)) file_handler.setFormatter(file_formatter) _logger.addHandler(file_handler) + if include_pyvisa_logs: + # Hook into pyvisa's logging to save it into the same file + pyvisa.logger.setLevel(getattr(logging, file_logging_level.value)) + pyvisa.logger.addHandler(file_handler) + # Log a few things to just the file _logger.debug("timezone==%s", get_localzone()) # pyright: ignore[reportUnknownArgumentType] _logger.debug("%s==%s", PACKAGE_NAME, importlib.metadata.version(PACKAGE_NAME)) if console_logging_level != LoggingLevels.NONE: - console_handler = colorlog.StreamHandler(stream=sys.stdout) - console_handler.setLevel(getattr(logging, console_logging_level.value)) - console_formatter = colorlog.ColoredFormatter( - "%(log_color)s" + logging_console_format_string, log_colors=colorlog.default_log_colors - ) + if colored_output: + console_handler = colorlog.StreamHandler(stream=sys.stdout) + console_formatter = colorlog.ColoredFormatter( + "%(log_color)s" + logging_console_format_string, + log_colors=colorlog.default_log_colors, + ) + else: + console_handler = logging.StreamHandler(stream=sys.stdout) + console_formatter = logging.Formatter(logging_console_format_string) console_formatter.default_msec_format = ( "%s.%06d" # Use 6 digits of precision for milliseconds ) + + console_handler.setLevel(getattr(logging, console_logging_level.value)) console_handler.setFormatter(console_formatter) _logger.addHandler(console_handler) + _logger_initialized = True return _logger diff --git a/tests/conftest.py b/tests/conftest.py index f8912fe9..e80a8c46 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,7 +1,9 @@ """Pytest configuration file.""" +import logging import os import socket +import sys from pathlib import Path from typing import Generator, List, Tuple @@ -24,6 +26,29 @@ UNIT_TEST_TIMEOUT = 50 +#################################################################################################### +# Configure the logging for the package that will run during unit tests +class _DynamicStreamHandler(logging.StreamHandler): # pyright: ignore[reportMissingTypeArgument] + def emit(self, record: logging.LogRecord) -> None: + self.stream = sys.stdout + super().emit(record) + + +_logger = configure_logging( + console_logging_level=LoggingLevels.NONE, + file_logging_level=LoggingLevels.NONE, +) +_unit_test_console_handler = _DynamicStreamHandler(stream=sys.stdout) +_unit_test_console_handler.setLevel(logging.DEBUG) +_unit_test_console_formatter = logging.Formatter("%(asctime)s - %(message)s") +_unit_test_console_formatter.default_msec_format = ( + "%s.%06d" # Use 6 digits of precision for milliseconds +) +_unit_test_console_handler.setFormatter(_unit_test_console_formatter) +_logger.addHandler(_unit_test_console_handler) +#################################################################################################### + + def mock_gethostbyname(address: str) -> str: """Mock the socket.gethostbyname function.""" try: @@ -70,7 +95,6 @@ def fixture_device_manager() -> Generator[DeviceManager, None, None]: Yields: The DeviceManager instance. """ - configure_logging(console_logging_level=LoggingLevels.DEBUG) print() # noqa: T201 with mock.patch( "socket.gethostbyname", mock.MagicMock(side_effect=mock_gethostbyname) diff --git a/tests/test_helpers.py b/tests/test_helpers.py index 850430af..d2285a80 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -132,11 +132,6 @@ def test_check_network_connection() -> None: assert "ping >> 127.0.0.1" in message assert "Response from ping >>" in message - stdout = StringIO() - with redirect_stdout(stdout): - assert check_network_connection("name", "127.0.0.1")[0] - assert stdout.getvalue() == "" - def test_check_port_connection() -> None: """Test checking a port connection.""" @@ -147,14 +142,10 @@ def test_check_port_connection() -> None: assert check_port_connection("name", "127.0.0.1", 80, timeout_seconds=1) message = stdout.getvalue() assert "(name) >> checking if port 80 is open on 127.0.0.1" in message - assert message.endswith("Result >> True\n") + assert message.endswith("(name) port 80 open = True\n") - stdout = StringIO() - with redirect_stdout(stdout), mock.patch( - "socket.socket.connect", mock.MagicMock(side_effect=socket.error("")) - ): + with mock.patch("socket.socket.connect", mock.MagicMock(side_effect=socket.error(""))): assert not check_port_connection("name", "127.0.0.1", 55555, timeout_seconds=1) - assert stdout.getvalue() == "" def test_sanitizing_enums() -> None: @@ -228,13 +219,8 @@ def test_create_and_check_visa_connection(capsys: pytest.CaptureFixture[str]) -> assert check_visa_connection(dev_config_3, SIMULATED_VISA_LIB, "dev_config_3") stdout = capsys.readouterr().out - assert "checking if a VISA connection can be made to " in stdout - assert "Result >> True" in stdout - - assert check_visa_connection(dev_config_3, SIMULATED_VISA_LIB, "dev_config_3") - stdout = capsys.readouterr().out - assert "checking if a VISA connection can be made to " not in stdout - assert "Result >> True" not in stdout + assert "(dev_config_3) >> checking if a VISA connection can be made to " in stdout + assert "(dev_config_3) VISA connected = True" in stdout with mock.patch("pyvisa.ResourceManager", mock.MagicMock(side_effect=visa.Error())): assert not check_visa_connection(dev_config_3, SIMULATED_VISA_LIB, "dev_config_3") diff --git a/tests/test_pi_device.py b/tests/test_pi_device.py index 5e639415..2923200f 100644 --- a/tests/test_pi_device.py +++ b/tests/test_pi_device.py @@ -14,24 +14,23 @@ def test_pi_control( # noqa: PLR0915 - device_manager: DeviceManager, capsys: pytest.CaptureFixture[str] + device_manager: DeviceManager, + capsys: pytest.CaptureFixture[str], + caplog: pytest.LogCaptureFixture, ) -> None: """Test generic PIControl functionality. Args: device_manager: The DeviceManager object. capsys: The captured stdout and stderr. + caplog: The captured log messages. """ scope: MSO2 = device_manager.add_scope("MSO22-HOSTNAME") assert scope._open() # noqa: SLF001 assert scope.query_binary("CURVE?") == [0.0] assert "Query Binary Values >> " in capsys.readouterr().out - assert scope.query_binary("CURVE?", verbose=False) == [0.0] - assert "Query Binary Values >> " not in capsys.readouterr().out assert scope.query_raw_binary("CURVE?") == b"#14\x00\x00\x00\x00\n" assert "Query Raw Binary >> " in capsys.readouterr().out - assert scope.query_raw_binary("CURVE?", verbose=False) == b"#14\x00\x00\x00\x00\n" - assert "Query Raw Binary >> " not in capsys.readouterr().out scope.factory_reset() assert "Write >> 'FACTORY'" in capsys.readouterr().out with pytest.raises(AssertionError) as error: @@ -48,8 +47,6 @@ def test_pi_control( # noqa: PLR0915 with pytest.raises(AssertionError): scope.query_less_than("*OPC?", 1) - scope.write_raw(b"FACTORY", verbose=False) - assert "Write Raw >> " not in capsys.readouterr().out with mock.patch( "pyvisa.resources.messagebased.MessageBasedResource.write_raw", mock.MagicMock(side_effect=visa.VisaIOError(123)), @@ -85,12 +82,7 @@ def test_pi_control( # noqa: PLR0915 stdout = capsys.readouterr().out assert f"Attempting to establish a VISA connection with {scope.resource_expression}" in stdout assert f"Successfully established a VISA connection with {scope.resource_expression} " in stdout - assert scope.wait_for_visa_connection( - 0.1, sleep_seconds=1, accept_immediate_connection=True, verbose=False - ) - stdout = capsys.readouterr().out - assert "Attempting to establish a VISA connection with " not in stdout - assert "Successfully established a VISA connection with " not in stdout + with pytest.raises(AssertionError): scope.wait_for_visa_connection(0.1, sleep_seconds=0, accept_immediate_connection=False) with mock.patch("pyvisa.ResourceManager", mock.MagicMock(side_effect=visa.Error())): @@ -106,17 +98,17 @@ def test_pi_control( # noqa: PLR0915 stdout = capsys.readouterr().out assert scope.visa_timeout != old_timeout assert scope.visa_timeout == 6000 - assert "VISA timeout set to: 6000ms" in stdout + assert "VISA timeout set to: 6000.000ms" in stdout # test a temporary verbose OFF with scope.temporary_verbose(False): assert scope.verbose != old_verbose # do something that would normally cause a printout scope.visa_timeout = 5000 - stdout = capsys.readouterr().out - assert stdout == "" assert scope.visa_timeout == 5000 + assert caplog.records[-1].levelname == "DEBUG" + assert caplog.records[-1].message.endswith("VISA timeout set to: 5000.000ms") stdout = capsys.readouterr().out - assert f"VISA timeout set to: {old_timeout}ms" in stdout + assert f"VISA timeout set to: {old_timeout}.000ms" in stdout assert scope.visa_timeout == old_timeout # Test closing a device that is powered off diff --git a/tests/test_rest_api_device.py b/tests/test_rest_api_device.py index babd41fb..1bd0a4a5 100644 --- a/tests/test_rest_api_device.py +++ b/tests/test_rest_api_device.py @@ -28,7 +28,7 @@ class CustomRestApiDevice(RESTAPIControl, Device): def _check_api_connection(self) -> bool: """Define abstract method _check_api_connection.""" - return self.get("/api", verbose=False, allow_errors=True)[0] + return self.get("/api", allow_errors=True)[0] def _close(self) -> None: """Define abstract method _close.""" diff --git a/tests/test_scopes.py b/tests/test_scopes.py index 9363d607..bc6b6d65 100644 --- a/tests/test_scopes.py +++ b/tests/test_scopes.py @@ -364,12 +364,15 @@ def test_exceptions(device_manager: DeviceManager) -> None: device_manager.remove_all_devices() -def test_tekscope70k(device_manager: DeviceManager, capsys: pytest.CaptureFixture[str]) -> None: +def test_tekscope70k( + device_manager: DeviceManager, + caplog: pytest.LogCaptureFixture, +) -> None: """Test the tekscope_70k implementation. Args: device_manager: The DeviceManager object. - capsys: The captured stdout and stderr. + caplog: The captured log messages. """ scope: TekScope5k7k70k = device_manager.add_scope("127.0.0.1") assert scope.ip_address == "127.0.0.1" @@ -378,27 +381,32 @@ def test_tekscope70k(device_manager: DeviceManager, capsys: pytest.CaptureFixtur assert scope.wait_for_network_connection( wait_time=0.05, sleep_seconds=1, accept_immediate_connection=True ) - assert ( - f"Successfully established a network connection with {scope.ip_address}" - in capsys.readouterr().out - ) - assert scope.wait_for_network_connection( - wait_time=0.05, sleep_seconds=1, accept_immediate_connection=True - ) - assert ( + assert caplog.records[-1].message.startswith( f"Successfully established a network connection with {scope.ip_address}" - not in capsys.readouterr().out ) + assert caplog.records[-1].levelname == "INFO" + + with scope.temporary_verbose(False): + assert scope.wait_for_network_connection( + wait_time=0.05, sleep_seconds=1, accept_immediate_connection=True + ) + assert caplog.records[-1].message.startswith( + f"Successfully established a network connection with {scope.ip_address}" + ) + assert caplog.records[-1].levelname == "DEBUG" + with mock.patch( "subprocess.check_output", mock.MagicMock(side_effect=subprocess.CalledProcessError(1, "")) ): assert not scope.wait_for_network_connection( wait_time=0.05, sleep_seconds=1, accept_immediate_connection=True ) - assert ( - f"Unable to establish a network connection with {scope.ip_address}" - in capsys.readouterr().out + assert caplog.records[-1].message == ( + f"Unable to establish a network connection with " + f"{scope.ip_address} after 0.05 seconds" ) + assert caplog.records[-1].levelname == "WARNING" + with pytest.raises(AssertionError): scope.wait_for_network_connection( wait_time=0.05, sleep_seconds=1, accept_immediate_connection=False diff --git a/tests/test_smu.py b/tests/test_smu.py index d1c72800..63135650 100644 --- a/tests/test_smu.py +++ b/tests/test_smu.py @@ -23,13 +23,16 @@ # pylint: disable=too-many-locals def test_smu( # noqa: PLR0915 - device_manager: DeviceManager, capsys: pytest.CaptureFixture[str] + device_manager: DeviceManager, + capsys: pytest.CaptureFixture[str], + caplog: pytest.LogCaptureFixture, ) -> None: """Test the SMU driver and TSP's IEEE commands. Args: device_manager: The DeviceManager object. capsys: The captured stdout and stderr. + caplog: The captured log messages. """ smu: SMU2601B = device_manager.add_smu("smu2601b-hostname", alias="smu-device") assert id(device_manager.get_smu(number_or_alias="smu-device")) == id(smu) @@ -187,15 +190,18 @@ def test_smu( # noqa: PLR0915 ) assert ( f"Successfully established a connection to port 4000 on {smu.ip_address}" - in capsys.readouterr().out - ) - assert smu.wait_for_port_connection( - 4000, wait_time=0.05, sleep_seconds=0, accept_immediate_connection=True + in caplog.records[-1].message ) + assert caplog.records[-1].levelname == "INFO" + with smu.temporary_verbose(False): + assert smu.wait_for_port_connection( + 4000, wait_time=0.05, sleep_seconds=0, accept_immediate_connection=True + ) assert ( f"Successfully established a connection to port 4000 on {smu.ip_address}" - not in capsys.readouterr().out + in caplog.records[-1].message ) + assert caplog.records[-1].levelname == "DEBUG" with pytest.raises(AssertionError): smu.wait_for_port_connection( 4000, @@ -207,10 +213,11 @@ def test_smu( # noqa: PLR0915 assert not smu.wait_for_port_connection( 4000, wait_time=0.05, sleep_seconds=0, accept_immediate_connection=True ) - assert ( - f"Successfully established a connection to port 4000 on {smu.ip_address}" - not in capsys.readouterr().out + assert caplog.records[-1].message == ( + f"Unable to establish a connection to port 4000 " + f"on {smu.ip_address} after 0.05 seconds" ) + assert caplog.records[-1].levelname == "WARNING" buffer = smu.get_buffers("smua.nvbuffer1") expected_buffer = {"smua.nvbuffer1": [1.0, 2.0, 3.0, 4.0, 5.0]} diff --git a/tests/test_verification_functions.py b/tests/test_verification_functions.py index 66a3506d..487a89b5 100644 --- a/tests/test_verification_functions.py +++ b/tests/test_verification_functions.py @@ -23,7 +23,7 @@ def test_verify_values_fail() -> None: log_error=True, ) assert ( - " - ERROR: (failing-check) : Actual result does not match the expected " + "ERROR: (failing-check) : Actual result does not match the expected " "result within a tolerance of 0.0, max: 0.1, act: 0.2, min: 0.1" ) in str(assertion_info.value) @@ -48,7 +48,7 @@ def test_verify_values_regex_match_fail() -> None: use_regex_match=True, ) assert ( - " - FAILURE: (regex-fail-check) : Actual result does not match the expected result, " + "FAILURE: (regex-fail-check) : Actual result does not match the expected result, " "exp: ^test.*value$, act: fail123value" ) in str(assertion_info.value) @@ -71,7 +71,7 @@ def test_verify_values_condense_printout(log_error: bool, message_level: str) -> log_error=log_error, ) assert ( - f" - {message_level}: (condense-printout-check) : " + f"{message_level}: (condense-printout-check) : " f"Actual result does not match the expected result" "\n exp: expected" "\n act: actual" From 5768640d597c07034bd6b12c739e1aa18ce9481a Mon Sep 17 00:00:00 2001 From: "Felt, Nicholas" Date: Fri, 8 Nov 2024 18:11:19 -0800 Subject: [PATCH 04/11] feat: Added individual VISA command logging for each VISA device. --- CHANGELOG.md | 6 +++ .../device_control/pi_control.py | 51 +++++++++++++++---- src/tm_devices/helpers/logging.py | 1 - tests/conftest.py | 4 +- tests/test_all_device_drivers.py | 20 ++++++++ tests/test_logging.py | 39 ++++++++++++++ 6 files changed, 109 insertions(+), 12 deletions(-) create mode 100644 tests/test_logging.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 6fdbd033..688b15f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,12 @@ Things to be included in the next release go here. - Added the `get_errors()` method to the `Device` class to enable easy access to the current error code and messages on any device. - Added more details to the Architectural Overview page of the documentation as well as highlighting to the device driver diagram on the page. - Added regex matching to the `verify_values()` helper function to allow for more flexible value verification. +- A main logfile is now created by default (can be disabled if desired) that contains all the logging output of the entire tm_devices package during execution. + - Use the `configure_logging()` function to set the logging levels for stdout and file logging. + - The default settings will log all messages to the log file and maintain the current printout functionality on stdout. +- A logfile is now created that contains each command sent to a VISA device. + - This file is located next to the main log file and will start with the same name, but have the unique address of the device appended. + - This file will only be created if file logging is enabled for the package (which is the default behavior). ### Changed diff --git a/src/tm_devices/driver_mixins/device_control/pi_control.py b/src/tm_devices/driver_mixins/device_control/pi_control.py index 1e5f8b97..58421b44 100644 --- a/src/tm_devices/driver_mixins/device_control/pi_control.py +++ b/src/tm_devices/driver_mixins/device_control/pi_control.py @@ -1,6 +1,8 @@ """Base Programmable Interface (PI) control class module.""" +import contextlib import logging +import logging.handlers import os import socket import time @@ -8,6 +10,7 @@ from abc import ABC from contextlib import contextmanager +from pathlib import Path from typing import final, Generator, List, Optional, Sequence, Tuple, Union import pyvisa as visa @@ -35,6 +38,7 @@ verify_values, ) from tm_devices.helpers import ReadOnlyCachedProperty as cached_property # noqa: N813 +from tm_devices.helpers.constants_and_dataclasses import PACKAGE_NAME _logger: logging.Logger = logging.getLogger(__name__) @@ -339,14 +343,13 @@ def query( # pylint: disable=arguments-differ Error: An error occurred while sending the command. SystemError: An empty string was returned from the device. """ - # TODO: logging: Log just the query to a device specific file _logger.log( logging.INFO if self._verbose and verbose else logging.DEBUG, "(%s) Query >> %r", self._name_and_alias, query, ) - + self._command_logger.debug(query) try: response = self._visa_resource.query(query).strip() if remove_quotes: @@ -386,14 +389,13 @@ def query_binary(self, query: str, verbose: bool = True) -> Sequence[float]: Error: An error occurred while sending the command. SystemError: An empty string was returned from the device. """ - # TODO: logging: Log just the query to a device specific file _logger.log( logging.INFO if self._verbose and verbose else logging.DEBUG, "(%s) Query Binary Values >> %r", self._name_and_alias, query, ) - + self._command_logger.debug(query) try: response = self._visa_resource.query_binary_values(query) # pyright: ignore[reportUnknownMemberType] except (visa.VisaIOError, socket.error) as error: @@ -516,14 +518,13 @@ def query_raw_binary(self, query: str, verbose: bool = True) -> bytes: Error: An error occurred while sending the command. SystemError: An empty string was returned from the device. """ - # TODO: logging: Log just the query to a device specific file _logger.log( logging.INFO if self._verbose and verbose else logging.DEBUG, "(%s) Query Raw Binary >> %r", self._name_and_alias, query, ) - + self._command_logger.debug(query) try: self._visa_resource.write(query) response = self.read_raw() @@ -841,7 +842,6 @@ def write(self, command: str, opc: bool = False, verbose: bool = True) -> None: Error: An error occurred while sending the command. SystemError: ``*OPC?`` did not return "1" after sending the command. """ - # TODO: logging: Log just the write to a device specific file if "\n" in command: _logger.log( logging.INFO if self._verbose and verbose else logging.DEBUG, @@ -856,7 +856,7 @@ def write(self, command: str, opc: bool = False, verbose: bool = True) -> None: self._name_and_alias, command, ) - + self._command_logger.debug(command) try: self._visa_resource.write(command) except (visa.VisaIOError, socket.error) as error: @@ -879,14 +879,13 @@ def write_raw(self, command: bytes, verbose: bool = True) -> None: Raises: Error: An error occurred while sending the command. """ - # TODO: logging: Log just the write to a device specific file _logger.log( logging.INFO if self._verbose and verbose else logging.DEBUG, "(%s) Write Raw >> %r", self._name_and_alias, command, ) - + self._command_logger.debug(command) try: self._visa_resource.write_raw(command) except (visa.VisaIOError, socket.error) as error: @@ -956,6 +955,38 @@ def _close(self) -> None: self._visa_resource = None # pyright: ignore[reportAttributeAccessIssue] self._is_open = False + @cached_property + def _command_logger(self) -> logging.Logger: + """Create a logger that will be used to log commands sent via write/query-like methods.""" + # Create the logger + command_logger = logging.getLogger(f"{self._config_entry.address}-visa_logger") + command_logger.setLevel(logging.DEBUG) + command_logger.propagate = False + command_logger.addHandler(logging.NullHandler()) + with contextlib.suppress(IndexError, StopIteration): + # Get the top-level log filepath + main_log_file = Path( + next( + x + for x in logging.getLogger(PACKAGE_NAME).handlers + if isinstance(x, logging.FileHandler) + ).baseFilename + ) + # Create the handler with the filename based on the main log file + command_log_handler = logging.FileHandler( + main_log_file.as_posix().replace( + main_log_file.suffix, f"_visa_commands_{self._config_entry.address}.log" + ), + mode="w", + encoding="utf-8", + ) + # Create the formatter + command_log_formatter = logging.Formatter("%(message)s") + command_log_handler.setFormatter(command_log_formatter) + command_log_handler.setLevel(logging.DEBUG) + command_logger.addHandler(command_log_handler) + return command_logger + def _open(self) -> bool: """Open necessary resources and components and return a boolean indicating success.""" opened = True diff --git a/src/tm_devices/helpers/logging.py b/src/tm_devices/helpers/logging.py index fdfe53c4..f9532b17 100644 --- a/src/tm_devices/helpers/logging.py +++ b/src/tm_devices/helpers/logging.py @@ -78,7 +78,6 @@ def configure_logging( log_file_name = f"{PACKAGE_NAME}_{time.strftime('%m-%d-%Y_%H-%M-%S', time.localtime())}.log" log_filepath = log_file_directory / log_file_name - # TODO: logging: look into rotating log files if file_logging_level != LoggingLevels.NONE: # Set up logger for tm_devices log_filepath.parent.mkdir(parents=True, exist_ok=True) diff --git a/tests/conftest.py b/tests/conftest.py index e80a8c46..5cd01248 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -36,7 +36,9 @@ def emit(self, record: logging.LogRecord) -> None: _logger = configure_logging( console_logging_level=LoggingLevels.NONE, - file_logging_level=LoggingLevels.NONE, + file_logging_level=LoggingLevels.DEBUG, + log_file_name="unit_test.log", + log_file_directory=Path(__file__).parent / "logs", ) _unit_test_console_handler = _DynamicStreamHandler(stream=sys.stdout) _unit_test_console_handler.setLevel(logging.DEBUG) diff --git a/tests/test_all_device_drivers.py b/tests/test_all_device_drivers.py index bacb1a98..d28afce6 100644 --- a/tests/test_all_device_drivers.py +++ b/tests/test_all_device_drivers.py @@ -4,6 +4,7 @@ import contextlib from collections import Counter +from pathlib import Path from typing import Generator, List, Optional import pytest @@ -209,3 +210,22 @@ def test_all_device_drivers() -> None: print( # noqa: T201 f"{len(drivers_with_auto_generated_commands)} device drivers have auto-generated commands" ) + + +@pytest.mark.order(3) +@pytest.mark.depends(on=["test_device_driver"]) +def test_visa_device_command_logging() -> None: + """Test the VISA command log file contents.""" + generated_file = Path(__file__).parent / "logs/unit_test_visa_commands_SMU2410-HOSTNAME.log" + assert generated_file.exists(), f"File not found: {generated_file}" + assert ( + generated_file.read_text() + == """*CLS +*RST +*OPC? +*ESR? +SYSTEM:ERROR? +*ESR? +SYSTEM:ERROR? +""" + ) diff --git a/tests/test_logging.py b/tests/test_logging.py new file mode 100644 index 00000000..176a026f --- /dev/null +++ b/tests/test_logging.py @@ -0,0 +1,39 @@ +"""Tests for the logging functionality.""" + +import contextlib +import logging + +from typing import Generator, TYPE_CHECKING + +import pytest + +from tm_devices import DeviceManager, PACKAGE_NAME + +if TYPE_CHECKING: + from tm_devices.drivers import MSO2 + + +@pytest.fixture(name="remove_log_file_handler") +def _remove_log_file_handler(device_manager: DeviceManager) -> Generator[None, None, None]: # pyright: ignore[reportUnusedFunction] + """Remove the file handler from the logger.""" + device_manager.remove_all_devices() + logger = logging.getLogger(PACKAGE_NAME) + file_handler = None + with contextlib.suppress(StopIteration): + file_handler = next( + handler for handler in logger.handlers if isinstance(handler, logging.FileHandler) + ) + logger.removeHandler(file_handler) + yield + if file_handler is not None: + logger.addHandler(file_handler) + device_manager.remove_all_devices() + + +def test_visa_command_logging_edge_cases( + device_manager: DeviceManager, + remove_log_file_handler: None, # noqa: ARG001 +) -> None: + """Test VISA command logging edge cases.""" + scope: MSO2 = device_manager.add_scope("MSO22-HOSTNAME") + assert scope.model == "MSO22" From 007c906422c47f8ea3d1f9c685f2211417b10869 Mon Sep 17 00:00:00 2001 From: "Felt, Nicholas" Date: Mon, 11 Nov 2024 09:55:59 -0800 Subject: [PATCH 05/11] refactor: Update linting settings and update code to use Path objects instead of builtins where possible. --- CHANGELOG.md | 5 ++- docs/macros.py | 4 +-- examples/scopes/tekscope/basic_curve_query.py | 6 ++-- pyproject.toml | 31 +++++++------------ scripts/contributor_setup.py | 4 +-- src/tm_devices/components/dm_config_parser.py | 2 +- src/tm_devices/device_manager.py | 4 ++- .../device_control/pi_control.py | 5 +-- .../device_control/tsp_control.py | 10 +++--- src/tm_devices/drivers/device.py | 6 ++-- .../drivers/margin_testers/margin_tester.py | 20 +++++++----- .../drivers/scopes/tekscope/tekscope.py | 4 +-- .../drivers/scopes/tekscope_2k/tekscope_2k.py | 12 +++++-- .../helpers/constants_and_dataclasses.py | 2 +- src/tm_devices/helpers/functions.py | 2 +- src/tm_devices/helpers/logging.py | 8 +++-- src/tm_devices/helpers/stubgen.py | 21 ++++++------- tests/conftest.py | 4 +-- tests/test_config_parser.py | 6 ++-- tests/test_device_manager.py | 8 ++--- tests/test_docs.py | 2 +- tests/test_extension_mixin.py | 2 +- tests/test_margin_testers.py | 4 +-- tests/test_scopes.py | 8 ++--- tests/test_smu.py | 8 ++--- 25 files changed, 99 insertions(+), 89 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 688b15f3..80dc5443 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,11 +54,12 @@ However, please read through all changes to be aware of what may potentially imp - Updated the auto-generated command mixin classes to no longer use an `__init__()` method to enable the driver API documentation to render in a more usable way. - Switched from using standard `print()` calls to using the `logging` module for all logging in the `tm_devices` package. - A configuration function provides the ability to set different logging levels for stdout and file logging. + - The config file and environment variable can also be used to control the logging functionality. - The debug logging from the `pyvisa` package is also included in the log file by default. ### Removed -- _**BREAKING CHANGE**_: Removed previously deprecated `TekScopeSW` alias to the `TekScopePC` class +- _**BREAKING CHANGE**_: Removed previously deprecated `TekScopeSW` alias to the `TekScopePC` class. - _**BREAKING CHANGE**_: Removed previously deprecated `write_buffers()` from the `TSPControl` class. - _**BREAKING CHANGE**_: Removed Internal AFG methods from the `TekScopePC` driver, since they wouldn't have worked due to its lack of an IAFG. - _**BREAKING CHANGE**_: Removed previously deprecated `DEVICE_DRIVER_MODEL_MAPPING` constant. @@ -66,6 +67,8 @@ However, please read through all changes to be aware of what may potentially imp - _**BREAKING CHANGE**_: Removed many hacky implementations of `total_channels` and `all_channel_names_list` properties from drivers that don't need them anymore. - _**BREAKING CHANGE**_: Removed the `verify_values()`, `raise_failure()`, and `raise_error()` methods from all device drivers. - These methods have been converted to helper functions and can be imported from the `tm_devices.helpers` subpackage now. +- _**BREAKING CHANGE**_: Removed the `print_with_timestamp()` function since this functionality is now handled by the `logging` module. +- _**BREAKING CHANGE**_: Removed the `get_timestamp_string()` function since this functionality is now handled by the `logging` module. --- diff --git a/docs/macros.py b/docs/macros.py index 8031460b..99c33017 100644 --- a/docs/macros.py +++ b/docs/macros.py @@ -265,8 +265,8 @@ def define_env(env: MacrosPlugin) -> None: used to perform a transformation """ # Read in the current package version number to use in templates and files - with open( - pathlib.Path(f"{pathlib.Path(__file__).parents[1]}") / "pyproject.toml", "rb" + with (pathlib.Path(f"{pathlib.Path(__file__).parents[1]}") / "pyproject.toml").open( + "rb" ) as file_handle: pyproject_data = tomli.load(file_handle) package_version = "v" + pyproject_data["tool"]["poetry"]["version"] diff --git a/examples/scopes/tekscope/basic_curve_query.py b/examples/scopes/tekscope/basic_curve_query.py index 219b1a8c..aeb70d6d 100644 --- a/examples/scopes/tekscope/basic_curve_query.py +++ b/examples/scopes/tekscope/basic_curve_query.py @@ -1,9 +1,11 @@ """An example showing a basic curve query.""" +from pathlib import Path + from tm_devices import DeviceManager from tm_devices.drivers import AFG3KC, MSO5 -EXAMPLE_CSV_FILE = "example_curve_query.csv" +EXAMPLE_CSV_FILE = Path("example_curve_query.csv") with DeviceManager(verbose=True) as dm: scope: MSO5 = dm.add_scope("MSO56-100083") @@ -16,7 +18,7 @@ curve_returned = scope.curve_query(1, output_csv_file=EXAMPLE_CSV_FILE) # Read in the curve query from file -with open(EXAMPLE_CSV_FILE, encoding="utf-8") as csv_content: +with EXAMPLE_CSV_FILE.open(encoding="utf-8") as csv_content: curve_saved = [int(i) for i in csv_content.read().split(",")] # Verify query saved to csv is the same as the one returned from curve_query function call diff --git a/pyproject.toml b/pyproject.toml index f4cfaaf6..49fb4400 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -319,26 +319,19 @@ fixable = ["ALL"] flake8-pytest-style = {mark-parentheses = false} flake8-quotes = {docstring-quotes = "double"} ignore = [ - "ANN101", # Missing type annotation for self in method - "ANN102", # Missing type annotation for cls in method - "ANN401", # Dynamically typed expressions (typing.Any) are disallowed in *args and **kwargs - "COM812", # Trailing comma missing - "EM102", # Exception must not use an f-string literal, assign to variable first - "FA100", # Missing `from __future__ import annotations`, but uses ... - "FBT", # flake8-boolean-trap - "FIX002", # Line contains TO DO + "ANN401", # Dynamically typed expressions (typing.Any) are disallowed in *args and **kwargs (allowed in this package) + "COM812", # Trailing comma missing (allowed in this package) + "FA100", # Missing `from __future__ import annotations`, but uses ... (allowed in this package) + "FBT", # flake8-boolean-trap # TODO: logging: remove this exclusion? + "FIX002", # Line contains TO DO (allowed in this package) "ISC001", # single-line-implicit-string-concatenation (handled by formatter) - "PTH109", # `os.getcwd()` should be replaced by `Path.cwd()` - "PTH123", # `open()` should be replaced by `Path.open()` - "PTH207", # Replace `iglob` with `Path.glob` or `Path.rglob` - "PYI021", # Docstrings should not be included in stubs - "TD002", # Missing author in TO DO - "TD003", # Missing issue link on the line following this TO DO - "TRY301", # Abstract raise to an inner function - "UP006", # Use {to} instead of {from} for type annotation - "UP007", # Use `X | Y` for type annotations - "UP024", # Replace aliased errors with `OSError` - "UP037" # Remove quotes from type annotation + "PYI021", # Docstrings should not be included in stubs (allowed in this package) + "TD002", # Missing author in TO DO (allowed in this package) + "TD003", # Missing issue link on the line following this TO DO (allowed in this package) + "UP006", # Use {to} instead of {from} for type annotation (allowed in this package) + "UP007", # Use `X | Y` for type annotations (allowed in this package) + "UP024", # Replace aliased errors with `OSError` (allowed in this package) + "UP037" # Remove quotes from type annotation (allowed in this package) ] pydocstyle = {convention = "google"} pylint = {max-args = 7} diff --git a/scripts/contributor_setup.py b/scripts/contributor_setup.py index fd38579f..a5bdca28 100644 --- a/scripts/contributor_setup.py +++ b/scripts/contributor_setup.py @@ -48,7 +48,7 @@ def main() -> None: starting_dir = Path.cwd() try: if RUNNING_IN_VIRTUALENV: - raise IndexError + raise IndexError # noqa: TRY301 # This requires contributors to use newer versions of Python even # though the package supports older versions. if sys.version_info < (3, 9): @@ -76,7 +76,7 @@ def main() -> None: files = list( filter( lambda x: "site-packages" not in x and "pythonw" not in x, - glob.iglob( + glob.iglob( # noqa: PTH207 f"{virtual_env_dir}/{'bin' if RUNNING_ON_LINUX else 'Scripts'}/**/python*", recursive=True, ), diff --git a/src/tm_devices/components/dm_config_parser.py b/src/tm_devices/components/dm_config_parser.py index 7c515c6e..e35d22c0 100644 --- a/src/tm_devices/components/dm_config_parser.py +++ b/src/tm_devices/components/dm_config_parser.py @@ -430,7 +430,7 @@ def __parse_config_file( if not config_path.is_file(): raise FileNotFoundError(config_path) # read in data - with open(config_path, encoding="utf-8") as config_file: + with config_path.open(encoding="utf-8") as config_file: if config_path.suffix == ".toml": data = tomli.loads(config_file.read()) else: # ["yaml", "yml"] diff --git a/src/tm_devices/device_manager.py b/src/tm_devices/device_manager.py index 6696cd28..ccfaa76c 100644 --- a/src/tm_devices/device_manager.py +++ b/src/tm_devices/device_manager.py @@ -136,7 +136,6 @@ def __init__( (updates the current configuration options). external_device_drivers: An optional dict for passing in additional device drivers. """ - configure_logging() self._suppress_protection = False # Set up the DeviceManager self.__is_open = False @@ -159,6 +158,9 @@ def __init__( # actually populate the options self.__set_options(verbose) + # TODO: logging: pass in necessary config options to the logger + configure_logging() + self.open() def __del__(self) -> None: diff --git a/src/tm_devices/driver_mixins/device_control/pi_control.py b/src/tm_devices/driver_mixins/device_control/pi_control.py index 58421b44..b55e94e8 100644 --- a/src/tm_devices/driver_mixins/device_control/pi_control.py +++ b/src/tm_devices/driver_mixins/device_control/pi_control.py @@ -250,7 +250,7 @@ def get_visa_stb(self) -> int: # pragma: no cover """Return the VISA status byte.""" return self._visa_resource.read_stb() - def poll_query( # noqa: PLR0913 + def poll_query( # noqa: PLR0913 # pylint: disable=too-many-locals self, number_of_polls: int, query: str, @@ -315,10 +315,11 @@ def poll_query( # noqa: PLR0913 return time.sleep(sleep_time) poll_number += 1 - raise AssertionError( # noqa: TRY003 + msg = ( f"{query} {'never' if not invert_range else 'always'} " f"returned {wanted_val}, received:\n{query_list}" ) + raise AssertionError(msg) def query( # pylint: disable=arguments-differ self, diff --git a/src/tm_devices/driver_mixins/device_control/tsp_control.py b/src/tm_devices/driver_mixins/device_control/tsp_control.py index a0253dab..26ff0caa 100644 --- a/src/tm_devices/driver_mixins/device_control/tsp_control.py +++ b/src/tm_devices/driver_mixins/device_control/tsp_control.py @@ -5,6 +5,7 @@ import logging from abc import ABC +from pathlib import Path from typing import Any, Dict, List, Optional, TYPE_CHECKING, Union from tm_devices.driver_mixins.device_control.pi_control import PIControl @@ -52,7 +53,9 @@ def ieee_cmds(self) -> TSPIEEE4882Commands: ################################################################################################ # Public Methods ################################################################################################ - def export_buffers(self, filepath: str, *args: str, sep: str = ",") -> None: + def export_buffers( + self, filepath: Union[str, os.PathLike[str]], *args: str, sep: str = "," + ) -> None: """Export one or more of the device's buffers to the given filepath. Args: @@ -60,7 +63,7 @@ def export_buffers(self, filepath: str, *args: str, sep: str = ",") -> None: args: The buffer name(s) to export. sep: The delimiter used to separate data. Defaults to ",". """ - with open(filepath, mode="w", encoding="utf-8") as file: + with Path(filepath).open(mode="w", encoding="utf-8") as file: buffer_data = self.get_buffers(*args) column_length = max(len(x) for x in buffer_data.values()) file.write(sep.join(buffer_data) + "\n") @@ -137,8 +140,7 @@ def load_script( """ if file_path is not None: # script_body argument is overwritten by file contents - with open(file_path, encoding="utf-8") as script_tsp: - script_body = script_tsp.read().strip() + script_body = Path(file_path).read_text(encoding="utf-8").strip() # Check if the script exists, delete it if it does self.write(f"if {script_name} ~= nil then script.delete('{script_name}') end") diff --git a/src/tm_devices/drivers/device.py b/src/tm_devices/drivers/device.py index 562222d8..9fe94c72 100644 --- a/src/tm_devices/drivers/device.py +++ b/src/tm_devices/drivers/device.py @@ -150,7 +150,7 @@ def _open(self) -> bool: def _reboot(self) -> None: """Perform the actual rebooting code.""" raise NotImplementedError( - f"``._reboot()`` is not yet implemented for the {self.__class__.__name__} driver" + f"``._reboot()`` is not yet implemented for the {self.__class__.__name__} driver", # noqa: EM102 ) ################################################################################################ @@ -181,7 +181,7 @@ def alias(self) -> str: def command_argument_constants(self) -> Any: """Return the device command argument constants.""" raise NotImplementedError( - f"The {self.__class__.__name__} driver does not have a Python API for its commands yet." + f"The {self.__class__.__name__} driver does not have a Python API for its commands yet." # noqa: EM102 ) @property @@ -216,7 +216,7 @@ def command_verification_enabled(self) -> bool: def commands(self) -> Any: """Return the device commands.""" raise NotImplementedError( - f"The {self.__class__.__name__} driver does not have a Python API for its commands yet." + f"The {self.__class__.__name__} driver does not have a Python API for its commands yet." # noqa: EM102 ) @cached_property diff --git a/src/tm_devices/drivers/margin_testers/margin_tester.py b/src/tm_devices/drivers/margin_testers/margin_tester.py index 27161f45..c4231705 100644 --- a/src/tm_devices/drivers/margin_testers/margin_tester.py +++ b/src/tm_devices/drivers/margin_testers/margin_tester.py @@ -4,7 +4,8 @@ import time from abc import ABC, abstractmethod -from typing import Any, Dict, Mapping, MutableMapping, Tuple +from pathlib import Path +from typing import Any, Dict, Mapping, MutableMapping, Tuple, Union from packaging.version import Version from requests.structures import CaseInsensitiveDict @@ -33,7 +34,7 @@ def __init__(self, config_entry: DeviceConfigEntry, verbose: bool) -> None: """ super().__init__(config_entry, verbose) - self._auth_token_file_path = "" + self._auth_token_file_path: Path = Path() ################################################################################################ # Abstract Cached Properties @@ -91,12 +92,12 @@ def device_type(self) -> str: @property def auth_token_file_path(self) -> str: """Return the path to the file containing the auth token.""" - return self._auth_token_file_path + return self._auth_token_file_path.as_posix() @auth_token_file_path.setter - def auth_token_file_path(self, value: str) -> None: + def auth_token_file_path(self, value: Union[str, os.PathLike[str]]) -> None: """Set the path to the file containing the auth token.""" - self._auth_token_file_path = value + self._auth_token_file_path = Path(value) ################################################################################################ # Public Methods @@ -119,7 +120,11 @@ def _generate_headers(self) -> Mapping[str, str]: Raises: AssertionError: Indicates that no auth token file is available. """ - if not self._auth_token_file_path: + if not ( + self._auth_token_file_path + and self._auth_token_file_path.exists() + and self._auth_token_file_path.is_file() + ): msg = ( "No auth token file set! Please set the ``.auth_token_file_path`` attribute " "to point to a file with an authorization token." @@ -127,8 +132,7 @@ def _generate_headers(self) -> Mapping[str, str]: raise AssertionError(msg) headers: MutableMapping[str, str] = CaseInsensitiveDict() headers["Accept"] = "application/json" - with open(self._auth_token_file_path, encoding="utf-8") as auth_file: - token = auth_file.read().replace("\n", "") + token = self._auth_token_file_path.read_text(encoding="utf-8").replace("\n", "") headers["Authorization"] = f"Bearer {token}" return headers diff --git a/src/tm_devices/drivers/scopes/tekscope/tekscope.py b/src/tm_devices/drivers/scopes/tekscope/tekscope.py index a618f39b..6f03fa24 100644 --- a/src/tm_devices/drivers/scopes/tekscope/tekscope.py +++ b/src/tm_devices/drivers/scopes/tekscope/tekscope.py @@ -327,7 +327,7 @@ def curve_query( # noqa: PLR0912,C901 self, channel_num: int, wfm_type: str = "TimeDomain", - output_csv_file: Optional[str] = None, + output_csv_file: Optional[Union[str, os.PathLike[str]]] = None, ) -> List[Any]: """Perform a curve query on a specific channel. @@ -397,7 +397,7 @@ def curve_query( # noqa: PLR0912,C901 wfm_data.append([float(b) for b in frame.split(",")]) # noqa: PERF401 if output_csv_file: - with open(output_csv_file, "w", encoding="UTF-8") as csv_file: + with Path(output_csv_file).open("w", encoding="UTF-8") as csv_file: for frame in frames: csv_file.write(frame) csv_file.write(",") diff --git a/src/tm_devices/drivers/scopes/tekscope_2k/tekscope_2k.py b/src/tm_devices/drivers/scopes/tekscope_2k/tekscope_2k.py index 87f3a3b5..8c2e53a2 100644 --- a/src/tm_devices/drivers/scopes/tekscope_2k/tekscope_2k.py +++ b/src/tm_devices/drivers/scopes/tekscope_2k/tekscope_2k.py @@ -1,10 +1,13 @@ """Base TekScope2k scope device driver module.""" +from __future__ import annotations + import logging import warnings from abc import ABC -from typing import Any, List, Optional +from pathlib import Path +from typing import Any, List, Optional, TYPE_CHECKING, Union from tm_devices.driver_mixins.abstract_device_functionality.channel_control_mixin import ( ChannelControlMixin, @@ -17,6 +20,9 @@ from tm_devices.drivers.scopes.scope import Scope from tm_devices.helpers import ReadOnlyCachedProperty as cached_property # noqa: N813 +if TYPE_CHECKING: + import os + _logger: logging.Logger = logging.getLogger(__name__) @@ -65,7 +71,7 @@ def curve_query( # pylint: disable=too-many-locals self, channel_num: int, wfm_type: str = "TimeDomain", - output_csv_file: Optional[str] = None, + output_csv_file: Optional[Union[str, os.PathLike[str]]] = None, ) -> List[Any]: """Perform a curve query on a specific channel. @@ -116,7 +122,7 @@ def curve_query( # pylint: disable=too-many-locals wfm_data = [round(i, 3) for i in data] if output_csv_file: - with open(output_csv_file, "w", encoding="UTF-8") as csv_file: + with Path(output_csv_file).open("w", encoding="UTF-8") as csv_file: for frame in wfm_data: csv_file.write(str(frame)) csv_file.write(",") diff --git a/src/tm_devices/helpers/constants_and_dataclasses.py b/src/tm_devices/helpers/constants_and_dataclasses.py index edda8fe7..703a25f4 100644 --- a/src/tm_devices/helpers/constants_and_dataclasses.py +++ b/src/tm_devices/helpers/constants_and_dataclasses.py @@ -306,7 +306,7 @@ def __post_init__(self) -> None: # noqa: PLR0912, C901 ) ) ): - raise ValueError + raise ValueError # noqa: TRY301 except ValueError as exc: msg = f"""Found invalid value of `address={self.address}` Must provide an valid address format for the `connection_type={self.connection_type.value}': diff --git a/src/tm_devices/helpers/functions.py b/src/tm_devices/helpers/functions.py index 93803c74..a461c5b9 100644 --- a/src/tm_devices/helpers/functions.py +++ b/src/tm_devices/helpers/functions.py @@ -524,7 +524,7 @@ def get_visa_backend(visa_lib_path: str) -> str: if visa_lib_path == "py": visa_name = "PyVISA-py" else: - raise KeyError + raise KeyError # noqa: TRY301 except KeyError: found_visa = False for visa_type in visa_backends: diff --git a/src/tm_devices/helpers/logging.py b/src/tm_devices/helpers/logging.py index f9532b17..69e73659 100644 --- a/src/tm_devices/helpers/logging.py +++ b/src/tm_devices/helpers/logging.py @@ -33,11 +33,15 @@ def configure_logging( file_logging_level: LoggingLevels = LoggingLevels.DEBUG, log_file_directory: Optional[Path] = None, log_file_name: Optional[str] = None, - colored_output: bool = True, + colored_output: bool = False, include_pyvisa_logs: bool = True, ) -> logging.Logger: # pragma: no cover """Configure the logging for this package. + !!! note + After this function is called once, if it is called again, it will not perform any + additional configuration. It will simply return the base logger for the package. + Args: console_logging_level: The logging level to set for the console. Defaults to INFO. Set to [`LoggingLevels.NONE`][tm_devices.helpers.enums.LoggingLevels.NONE] to disable all @@ -49,7 +53,7 @@ def configure_logging( current working directory. log_file_name: The name of the log file to save the logs to. Defaults to a timestamped name. colored_output: Whether to use colored output from the `colorlog` package for the console. - Defaults to True. + Defaults to False. include_pyvisa_logs: Whether to include logs from the `pyvisa` package in the log file. The logging level will match the `file_logging_level`. Defaults to True. diff --git a/src/tm_devices/helpers/stubgen.py b/src/tm_devices/helpers/stubgen.py index 3c7591f6..ecd7f1ef 100644 --- a/src/tm_devices/helpers/stubgen.py +++ b/src/tm_devices/helpers/stubgen.py @@ -18,7 +18,7 @@ def _get_data_type(data_object: Any) -> str: """ try: if "." in str(data_object): - raise AttributeError + raise AttributeError # noqa: TRY301 data_type = str(data_object.__name__) if data_object else str(data_object) except AttributeError: data_type = str( @@ -46,15 +46,14 @@ def add_info_to_stub(cls: Any, method: Any, is_property: bool = False) -> None: """ if stub_dir := os.getenv("TM_DEVICES_STUB_DIR"): method_filepath = inspect.getfile(cls) - stub_dir = str( - Path(stub_dir) / "tm_devices" if not stub_dir.endswith("tm_devices") else stub_dir + stub_dir_path = Path(stub_dir) / ( + "tm_devices" if not stub_dir.endswith("tm_devices") else stub_dir ) - method_filepath = str( - Path(stub_dir) - / method_filepath.rsplit("tm_devices", maxsplit=1)[-1].lstrip(os.path.sep) + # stub files have the .pyi extension + method_path_obj = stub_dir_path / ( + method_filepath.rsplit("tm_devices", maxsplit=1)[-1].lstrip(os.path.sep) + "i" ) - method_filepath += "i" # stub files have the .pyi extension - if not os.path.exists(method_filepath): # noqa: PTH110 + if not method_path_obj.exists(): msg = ( f'The stub file "{method_filepath}" must already exist in order to use this ' f"functionality to add method stubs." @@ -86,8 +85,7 @@ def add_info_to_stub(cls: Any, method: Any, is_property: bool = False) -> None: method_signature = " @property\n" + method_signature method_stub_content = method_signature + " " + '"""' + method.__doc__ + '"""' + "\n" # Read in the content of the stub file to avoid adding duplicate methods - with open(method_filepath, encoding="utf-8") as file_pointer: - contents = file_pointer.read() + contents = method_path_obj.read_text(encoding="utf-8") if f" def {method.__name__}(" not in contents: if typing_imports: contents = f"from typing import {', '.join(typing_imports)}\n" + contents @@ -105,5 +103,4 @@ def add_info_to_stub(cls: Any, method: Any, is_property: bool = False) -> None: msg = f"Could not find the end of the {cls.__name__} class." raise ValueError(msg) - with open(method_filepath, "w", encoding="utf-8") as file_pointer: - file_pointer.write(contents) + method_path_obj.write_text(contents, encoding="utf-8") diff --git a/tests/conftest.py b/tests/conftest.py index 5cd01248..e1f2be20 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -57,7 +57,7 @@ def mock_gethostbyname(address: str) -> str: is_hostname = validate_address(address) if not is_hostname: # pylint: disable=consider-using-assignment-expr return address - raise ValueError + raise ValueError # noqa: TRY301 except ValueError as error: raise socket.herror from error @@ -68,7 +68,7 @@ def mock_gethostbyaddr(address: str) -> Tuple[str, List[str], List[str]]: is_hostname = validate_address(address) if is_hostname: # pylint: disable=consider-using-assignment-expr return address, [], [] - raise ValueError + raise ValueError # noqa: TRY301 except ValueError as error: raise socket.herror from error diff --git a/tests/test_config_parser.py b/tests/test_config_parser.py index a96f1589..dc00091e 100644 --- a/tests/test_config_parser.py +++ b/tests/test_config_parser.py @@ -179,7 +179,7 @@ def test_file_config_default_path() -> None: } with mock.patch.dict("os.environ", {}, clear=True), mock.patch( "pathlib.Path.is_file", mock.MagicMock(return_value=True) - ), mock.patch("builtins.open", mock.mock_open(read_data=file_contents)): + ), mock.patch("pathlib.Path.open", mock.mock_open(read_data=file_contents)): config = DMConfigParser() assert expected_options == config.options @@ -251,9 +251,7 @@ def test_file_config_non_default_path( config_2.load_config_file(os_environ["TM_DEVICES_CONFIG"]) # Read in the golden files - with open(os_environ["TM_DEVICES_CONFIG"], encoding="utf-8") as config_file: - text = config_file.read() - + text = Path(os_environ["TM_DEVICES_CONFIG"]).read_text(encoding="utf-8") assert config.to_config_file_text(file_type) == text, "issue generating config file text" assert config_2.to_config_file_text(file_type) == text, "issue generating config file text" diff --git a/tests/test_device_manager.py b/tests/test_device_manager.py index 38f35f00..6a88d5d8 100644 --- a/tests/test_device_manager.py +++ b/tests/test_device_manager.py @@ -69,8 +69,7 @@ def test_get_config_methods_with_devices(self, device_manager: DeviceManager) -> device_manager.write_current_configuration_to_config_file() # respect the env var if no path is given device_manager.write_current_configuration_to_config_file("./temp_config.yaml") - with open("./temp_config.toml", encoding="utf-8") as temp_config: - text = temp_config.read() + text = Path("./temp_config.toml").read_text(encoding="utf-8") assert ( text == f"""[[devices]] @@ -101,9 +100,8 @@ def test_get_config_methods_with_devices(self, device_manager: DeviceManager) -> verbose_visa = false """ ) - with open("./temp_config.yaml", encoding="utf-8") as temp_config: - text = temp_config.read() - print(text) # noqa: T201 + text = Path("./temp_config.yaml").read_text(encoding="utf-8") + print(text) # noqa: T201 assert ( text == f"""--- diff --git a/tests/test_docs.py b/tests/test_docs.py index 66e2a133..f9713c9e 100644 --- a/tests/test_docs.py +++ b/tests/test_docs.py @@ -51,7 +51,7 @@ def fixture_site_dir(pytestconfig: pytest.Config) -> str: @pytest.fixture(scope="module", autouse=True) def _docs_tests_setup() -> Generator[None, None, None]: # pyright: ignore [reportUnusedFunction] """Setup for docs tests..""" - starting_directory = os.getcwd() + starting_directory = Path.cwd() try: os.chdir(PROJECT_ROOT_DIR) yield diff --git a/tests/test_extension_mixin.py b/tests/test_extension_mixin.py index 96f2b7ec..b261ca67 100644 --- a/tests/test_extension_mixin.py +++ b/tests/test_extension_mixin.py @@ -243,7 +243,7 @@ def custom_model_getter_afg3kc(device: AFG3KC, value: str) -> str: return f"AFG3KC {device.model} {value}" ############################################################################################ - start_dir = os.getcwd() + start_dir = Path.cwd() try: os.chdir(generated_stub_dir) subprocess.check_call( # noqa: S603 diff --git a/tests/test_margin_testers.py b/tests/test_margin_testers.py index c588894c..f38d2f27 100644 --- a/tests/test_margin_testers.py +++ b/tests/test_margin_testers.py @@ -12,7 +12,7 @@ from tm_devices import DeviceManager from tm_devices.drivers.margin_testers.margin_tester import MarginTester -AUTH_TOKEN_FILE_PATH = f"{Path(__file__).parent}/samples/token.auth_token_file_path" # nosec +AUTH_TOKEN_FILE_PATH = Path(__file__).parent / "samples/token.auth_token_file_path" # nosec ################################################################################################ @@ -121,6 +121,6 @@ def test_generate_headers(tmt4: MarginTester) -> None: tmt4: Margin Tester device. """ tmt4.auth_token_file_path = AUTH_TOKEN_FILE_PATH - assert tmt4.auth_token_file_path == AUTH_TOKEN_FILE_PATH + assert tmt4.auth_token_file_path == AUTH_TOKEN_FILE_PATH.as_posix() headers = tmt4._generate_headers() # noqa: SLF001 assert headers["Authorization"] == "Bearer UNIT-TEST" diff --git a/tests/test_scopes.py b/tests/test_scopes.py index bc6b6d65..40b29941 100644 --- a/tests/test_scopes.py +++ b/tests/test_scopes.py @@ -155,10 +155,10 @@ def test_tekscope(device_manager: DeviceManager) -> None: # noqa: PLR0915 scope.expect_esr(0, ('0,"No events to report - queue empty"',)) # Test curve query write to csv functionality with multi-frame curve - filepath = f"temp_{sys.version_info.major}{sys.version_info.minor}.csv" + filepath = pathlib.Path(f"temp_{sys.version_info.major}{sys.version_info.minor}.csv") try: - curve = scope.curve_query(1, wfm_type="TimeDomain", output_csv_file=filepath) - with open(filepath, encoding="utf8") as curve_csv: + curve = scope.curve_query(1, wfm_type="TimeDomain", output_csv_file=filepath.as_posix()) + with filepath.open(encoding="utf8") as curve_csv: # Remove trailing command a format string as list of ints based on commas curve_reformatted_from_file = list(map(int, curve_csv.read()[:-1].split(","))) # Flatten list of lists returned from multi-frame curve query @@ -589,7 +589,7 @@ def test_tekscope2k(device_manager: DeviceManager, tmp_path: pathlib.Path) -> No filename = tmp_path / "temp.txt" curve = scope.curve_query(1, wfm_type="TimeDomain", output_csv_file=str(filename)) - with open(filename, encoding="utf8") as curve_csv: + with filename.open(encoding="utf8") as curve_csv: curve_reformatted_from_file = list(map(float, curve_csv.read()[:-1].split(","))) assert curve == curve_reformatted_from_file diff --git a/tests/test_smu.py b/tests/test_smu.py index 63135650..68d468a1 100644 --- a/tests/test_smu.py +++ b/tests/test_smu.py @@ -233,12 +233,12 @@ def test_smu( # noqa: PLR0915 for value in (1.0, 2.0, 3.0, 4.0, 5.0): assert str(value) in stdout - filepath = f"./temp_test_{sys.version_info.major}{sys.version_info.minor}.csv" + filepath = Path(f"./temp_test_{sys.version_info.major}{sys.version_info.minor}.csv") try: - smu.export_buffers(filepath, "smua.nvbuffer1") - assert os.path.exists(filepath) # noqa: PTH110 - with open(filepath, encoding="utf-8") as file: + smu.export_buffers(filepath.as_posix(), "smua.nvbuffer1") + assert filepath.exists() + with filepath.open(encoding="utf-8") as file: lines = file.readlines() for index, value in enumerate(["smua.nvbuffer1", "1.0", "2.0", "3.0", "4.0", "5.0"]): assert value in lines[index] From cd286559f4737290e4412646df25b49fb3e86ad2 Mon Sep 17 00:00:00 2001 From: "Felt, Nicholas" Date: Mon, 11 Nov 2024 16:29:34 -0800 Subject: [PATCH 06/11] docs: Updated documentation with instructions and examples on how to configure logging. --- docs/basic_usage.md | 17 ++++ docs/configuration.md | 47 ++++++++++- docs/key_features.md | 1 + docs/known_words.txt | 3 + examples/miscellaneous/customize_logging.py | 21 +++++ pyproject.toml | 3 +- src/tm_devices/__init__.py | 4 +- .../commands/gen_e3e9uu_lpdmso/pilogger.py | 8 +- .../commands/helpers/scpi_commands.py | 4 +- src/tm_devices/device_manager.py | 9 +- .../_abstract_device_control.py | 2 +- ...bstract_device_visa_write_query_control.py | 4 +- .../device_control/pi_control.py | 5 +- .../_tektronix_pi_scope_mixin.py | 2 +- .../common_pi_system_error_check_mixin.py | 2 +- .../common_tsp_error_check_mixin.py | 2 +- .../data_acquisition_systems/daq6510.py | 2 +- src/tm_devices/drivers/device.py | 6 +- .../drivers/digital_multimeters/dmm6500.py | 2 +- .../digital_multimeters/dmm75xx/dmm75xx.py | 2 +- .../drivers/margin_testers/margin_tester.py | 2 +- .../smu24xx/smu24xx_interactive.py | 2 +- src/tm_devices/helpers/__init__.py | 3 +- .../helpers/constants_and_dataclasses.py | 62 ++++++++++++++ src/tm_devices/helpers/enums.py | 17 ---- src/tm_devices/helpers/functions.py | 2 +- src/tm_devices/helpers/logging.py | 83 ++++++++++++------- tests/conftest.py | 6 +- tests/samples/sample_devices.toml | 1 + tests/samples/sample_devices.yaml | 1 + tests/test_config_parser.py | 15 +++- tests/validate_device_manager_delete.py | 2 +- 32 files changed, 257 insertions(+), 85 deletions(-) create mode 100644 examples/miscellaneous/customize_logging.py diff --git a/docs/basic_usage.md b/docs/basic_usage.md index 47d1c957..1c448c5d 100644 --- a/docs/basic_usage.md +++ b/docs/basic_usage.md @@ -59,6 +59,23 @@ outside the Python code for ease of automation --8<-- "examples/miscellaneous/adding_devices_with_env_var.py" ``` +## Customize logging and console output + +The amount of console output and logging saved to the log file can be customized as needed. This +configuration can be done in the Python code itself as demonstrated here, or by using the +[config file](configuration.md#config-options) or +[environment variable](configuration.md#environment-variable). + +!!! important + If any configuration is performed in the Python code prior to instantiating the + [`DeviceManager`][tm_devices.DeviceManager], all other logging configuration methods + (config file, env var) will be ignored. + +```python +# fmt: off +--8<-- "examples/miscellaneous/customize_logging.py" +``` + ## Disable command checking This removes an extra query that verifies the property was set to the expected diff --git a/docs/configuration.md b/docs/configuration.md index b9bce014..ab0bfd32 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -215,8 +215,7 @@ devices: ### Config Options -These options are flags that enable/disable runtime behaviors of the Device -Manager. +These options are used to configure runtime behaviors of `tm_devices`. #### Yaml Options Syntax @@ -230,6 +229,12 @@ options: retry_visa_connection: false default_visa_timeout: 5000 check_for_updates: false + log_console_level: INFO + log_file_level: DEBUG + log_file_directory: ./logs + log_file_name: tm_devices_.log + log_colored_output: false + log_pyvisa_messages: false ``` These are all `false` by default if not defined, set to `true` to modify the @@ -259,6 +264,30 @@ runtime behavior configuration. - `check_for_updates` - This config option will enable a check for any available updates on pypi.org for the package when the `DeviceManager` is instantiated. +- `log_console_level` + - This config option is used to set the log level for the console output. + The default value of this config option is "INFO". See the + [`configure_logging()`][tm_devices.helpers.logging.configure_logging] function for more information. +- `log_file_level` + - This config option is used to set the log level for the file output. + The default value of this config option is "DEBUG". See the + [`configure_logging()`][tm_devices.helpers.logging.configure_logging] function for more information. +- `log_file_directory` + - This config option is used to set the directory where the log files will be saved. + The default value of this config option is "./logs". See the + [`configure_logging()`][tm_devices.helpers.logging.configure_logging] function for more information. +- `log_file_name` + - This config option is used to set the name of the log file. + The default value of this config option is a timestamped filename with the .log extension. See the + [`configure_logging()`][tm_devices.helpers.logging.configure_logging] function for more information. +- `log_colored_output` + - This config option is used to enable or disable colored output in the console. + The default value of this config option is false. See the + [`configure_logging()`][tm_devices.helpers.logging.configure_logging] function for more information. +- `log_pyvisa_messages` + - This config option is used to enable or disable logging of PyVISA messages within the + configured log file. The default value of this config option is false. See the + [`configure_logging()`][tm_devices.helpers.logging.configure_logging] function for more information. ### Sample Config File @@ -320,6 +349,12 @@ options: retry_visa_connection: false default_visa_timeout: 10000 # 10 second default VISA timeout check_for_updates: false + log_console_level: NONE # completely disable console output + log_file_level: DEBUG + log_file_directory: ./logs + log_file_name: custom_logfile.log # customize the log file name + log_colored_output: false + log_pyvisa_messages: true # log PyVISA messages in the log file ``` #### TOML @@ -393,8 +428,14 @@ standalone = false verbose_mode = false verbose_visa = false retry_visa_connection = false -default_visa_timeout = 10000 # 10 second default VISA timeout +default_visa_timeout = 10000 # 10 second default VISA timeout check_for_updates = false +log_console_level = "NONE" # completely disable console output +log_file_level = "DEBUG" +log_file_directory = "./logs" +log_file_name = "custom_logfile.log" # customize the log file name +log_colored_output = false +log_pyvisa_messages = true # log PyVISA messages in the log file ``` --- diff --git a/docs/key_features.md b/docs/key_features.md index 1fa1f161..555b5dc8 100644 --- a/docs/key_features.md +++ b/docs/key_features.md @@ -14,3 +14,4 @@ complex examples. IntelliSense. - Organize connections to an entire "Test Bench" of devices with one package! - Full support for all VISA connection types (some require external drivers). +- Customizable logging to both the console and a log file. diff --git a/docs/known_words.txt b/docs/known_words.txt index 583a3f61..b90d6485 100644 --- a/docs/known_words.txt +++ b/docs/known_words.txt @@ -19,11 +19,13 @@ ci classdiagram codebase codecov +colored config conftest cookiecutter cov csv +customizable deps dev disable_command_verification @@ -33,6 +35,7 @@ docstring docstrings en enum +env executables filepath generate_function diff --git a/examples/miscellaneous/customize_logging.py b/examples/miscellaneous/customize_logging.py new file mode 100644 index 00000000..618a1b0d --- /dev/null +++ b/examples/miscellaneous/customize_logging.py @@ -0,0 +1,21 @@ +"""The console output and level of logging outputs in the log file can be configured as needed.""" + +from tm_devices import configure_logging, DeviceManager, LoggingLevels +from tm_devices.drivers import MSO6B + +# NOTE: This configuration will prevent any logging config options from a config file or +# environment variable from being used. +configure_logging( + log_console_level=LoggingLevels.NONE, # completely disable console logging + log_file_level=LoggingLevels.DEBUG, # log everything to the file + log_file_directory="./log_files", # save the log file in the "./log_files" directory + log_file_name="custom_log_filename.log", # customize the filename + log_pyvisa_messages=True, # include all the pyvisa debug messages in the same log file +) + +with DeviceManager(verbose=False) as dm: + scope: MSO6B = dm.add_scope("192.168.1.5") + scope.curve_query(1) + scope.check_port_connection(4000) + scope.check_network_connection() + scope.check_visa_connection() diff --git a/pyproject.toml b/pyproject.toml index 49fb4400..c63883b2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -322,7 +322,7 @@ ignore = [ "ANN401", # Dynamically typed expressions (typing.Any) are disallowed in *args and **kwargs (allowed in this package) "COM812", # Trailing comma missing (allowed in this package) "FA100", # Missing `from __future__ import annotations`, but uses ... (allowed in this package) - "FBT", # flake8-boolean-trap # TODO: logging: remove this exclusion? + "FBT", # flake8-boolean-trap # TODO: remove this exclusion? "FIX002", # Line contains TO DO (allowed in this package) "ISC001", # single-line-implicit-string-concatenation (handled by formatter) "PYI021", # Docstrings should not be included in stubs (allowed in this package) @@ -353,6 +353,7 @@ order-by-type = false [tool.ruff.lint.per-file-ignores] "examples/**" = [ + "FBT", # flake8-boolean-trap "S101", # Use of assert detected "T201" # `print` found ] diff --git a/src/tm_devices/__init__.py b/src/tm_devices/__init__.py index 36248516..82f8e70d 100644 --- a/src/tm_devices/__init__.py +++ b/src/tm_devices/__init__.py @@ -19,9 +19,9 @@ PYVISA_PY_BACKEND, SYSTEM_DEFAULT_VISA_BACKEND, ) -from tm_devices.helpers.enums import LoggingLevels, SupportedModels +from tm_devices.helpers.enums import SupportedModels from tm_devices.helpers.functions import register_additional_usbtmc_mapping -from tm_devices.helpers.logging import configure_logging +from tm_devices.helpers.logging import configure_logging, LoggingLevels # Read version from installed package. __version__ = version(PACKAGE_NAME) diff --git a/src/tm_devices/commands/gen_e3e9uu_lpdmso/pilogger.py b/src/tm_devices/commands/gen_e3e9uu_lpdmso/pilogger.py index 37f84b2b..c658948a 100644 --- a/src/tm_devices/commands/gen_e3e9uu_lpdmso/pilogger.py +++ b/src/tm_devices/commands/gen_e3e9uu_lpdmso/pilogger.py @@ -47,8 +47,8 @@ class PiloggerState(SCPICmdWrite, SCPICmdRead): Info: - ```` = 0 disables the PI command logger; any other value turns the PI command logger on. - - ``OFF`` disables the PI command _logger. - - ``ON`` enables the PI command _logger. + - ``OFF`` disables the PI command logger. + - ``ON`` enables the PI command logger. """ @@ -152,7 +152,7 @@ def state(self) -> PiloggerState: Info: - ```` = 0 disables the PI command logger; any other value turns the PI command logger on. - - ``OFF`` disables the PI command _logger. - - ``ON`` enables the PI command _logger. + - ``OFF`` disables the PI command logger. + - ``ON`` enables the PI command logger. """ return self._state diff --git a/src/tm_devices/commands/helpers/scpi_commands.py b/src/tm_devices/commands/helpers/scpi_commands.py index 99c73a68..7980d004 100644 --- a/src/tm_devices/commands/helpers/scpi_commands.py +++ b/src/tm_devices/commands/helpers/scpi_commands.py @@ -76,7 +76,7 @@ def verify(self, value: Union[float, str]) -> Tuple[bool, str]: Returns: A tuple containing a boolean indicating if the values match and a string with the actual - return value from the device. + return value from the device. Raises: tm_devices.commands.NoDeviceProvidedError: Indicates that no device connection exists. @@ -136,7 +136,7 @@ def verify(self, argument: str, value: Union[float, str]) -> Tuple[bool, str]: Returns: A tuple containing a boolean indicating if the values match and a string with the actual - return value from the device. + return value from the device. Raises: tm_devices.commands.NoDeviceProvidedError: Indicates that no device connection exists. diff --git a/src/tm_devices/device_manager.py b/src/tm_devices/device_manager.py index ccfaa76c..58b4647f 100644 --- a/src/tm_devices/device_manager.py +++ b/src/tm_devices/device_manager.py @@ -158,8 +158,13 @@ def __init__( # actually populate the options self.__set_options(verbose) - # TODO: logging: pass in necessary config options to the logger - configure_logging() + # Pass in the options from the config file or environment variable to the logger + logging_options = { + key: value + for key, value in self.__config.options.to_dict(ignore_none=True).items() + if key.startswith("log_") + } + configure_logging(**logging_options) self.open() diff --git a/src/tm_devices/driver_mixins/device_control/_abstract_device_control.py b/src/tm_devices/driver_mixins/device_control/_abstract_device_control.py index e6d19297..2030bc07 100644 --- a/src/tm_devices/driver_mixins/device_control/_abstract_device_control.py +++ b/src/tm_devices/driver_mixins/device_control/_abstract_device_control.py @@ -43,5 +43,5 @@ def _get_errors(self) -> Tuple[int, Tuple[str, ...]]: Returns: A tuple containing the current error code alongside a tuple of the current error - messages. + messages. """ diff --git a/src/tm_devices/driver_mixins/device_control/_abstract_device_visa_write_query_control.py b/src/tm_devices/driver_mixins/device_control/_abstract_device_visa_write_query_control.py index 5676ce36..da654c4d 100644 --- a/src/tm_devices/driver_mixins/device_control/_abstract_device_visa_write_query_control.py +++ b/src/tm_devices/driver_mixins/device_control/_abstract_device_visa_write_query_control.py @@ -43,8 +43,8 @@ def expect_esr( Returns: A boolean indicating if the check passed or failed, True means the check passed, - False means the check failed (however, failing the check will always result in an - AssertionError being raised, so the result will not really be usable). + False means the check failed (however, failing the check will always result in an + AssertionError being raised, so the result will not really be usable). Raises: AssertionError: Indicating that the device's error code and messages don't match the diff --git a/src/tm_devices/driver_mixins/device_control/pi_control.py b/src/tm_devices/driver_mixins/device_control/pi_control.py index b55e94e8..fe4a92ed 100644 --- a/src/tm_devices/driver_mixins/device_control/pi_control.py +++ b/src/tm_devices/driver_mixins/device_control/pi_control.py @@ -9,7 +9,6 @@ import warnings from abc import ABC -from contextlib import contextmanager from pathlib import Path from typing import final, Generator, List, Optional, Sequence, Tuple, Union @@ -197,7 +196,7 @@ def visa_backend(self) -> str: ################################################################################################ # Context Manager Methods ################################################################################################ - @contextmanager + @contextlib.contextmanager def temporary_visa_timeout(self, temporary_timeout_ms: float) -> Generator[None, None, None]: """Set a temporary VISA timeout value for the duration of the context. @@ -725,7 +724,7 @@ def set_if_needed( # noqa: PLR0913 Returns: Tuple containing the boolean value indicating if the command needed to be set and - the value returned from the query. + the value returned from the query. """ try: query_passed, actual_value = self.query_response( diff --git a/src/tm_devices/driver_mixins/shared_implementations/_tektronix_pi_scope_mixin.py b/src/tm_devices/driver_mixins/shared_implementations/_tektronix_pi_scope_mixin.py index 4b20b20f..17e24547 100644 --- a/src/tm_devices/driver_mixins/shared_implementations/_tektronix_pi_scope_mixin.py +++ b/src/tm_devices/driver_mixins/shared_implementations/_tektronix_pi_scope_mixin.py @@ -30,7 +30,7 @@ def _get_errors(self) -> Tuple[int, Tuple[str, ...]]: Returns: A tuple containing the current error code alongside a tuple of the current error - messages. + messages. """ result = int(self.query("*ESR?").strip()) allev_list = [ diff --git a/src/tm_devices/driver_mixins/shared_implementations/common_pi_system_error_check_mixin.py b/src/tm_devices/driver_mixins/shared_implementations/common_pi_system_error_check_mixin.py index cd153154..40874c95 100644 --- a/src/tm_devices/driver_mixins/shared_implementations/common_pi_system_error_check_mixin.py +++ b/src/tm_devices/driver_mixins/shared_implementations/common_pi_system_error_check_mixin.py @@ -30,7 +30,7 @@ def _get_errors(self) -> Tuple[int, Tuple[str, ...]]: Returns: A tuple containing the current error code alongside a tuple of the current error - messages. + messages. """ result = int(self.query("*ESR?").strip()) diff --git a/src/tm_devices/driver_mixins/shared_implementations/common_tsp_error_check_mixin.py b/src/tm_devices/driver_mixins/shared_implementations/common_tsp_error_check_mixin.py index 6ff784d7..184fdea0 100644 --- a/src/tm_devices/driver_mixins/shared_implementations/common_tsp_error_check_mixin.py +++ b/src/tm_devices/driver_mixins/shared_implementations/common_tsp_error_check_mixin.py @@ -25,7 +25,7 @@ def _get_errors(self) -> Tuple[int, Tuple[str, ...]]: Returns: A tuple containing the current error code alongside a tuple of the current error - messages. + messages. """ # instrument returns exponential numbers so converting to float before int error_code = int(float(self.query("print(status.standard.event)"))) diff --git a/src/tm_devices/drivers/data_acquisition_systems/daq6510.py b/src/tm_devices/drivers/data_acquisition_systems/daq6510.py index 827b7cfb..ca4d0c64 100644 --- a/src/tm_devices/drivers/data_acquisition_systems/daq6510.py +++ b/src/tm_devices/drivers/data_acquisition_systems/daq6510.py @@ -55,7 +55,7 @@ def _get_errors(self) -> Tuple[int, Tuple[str, ...]]: Returns: A tuple containing the current error code alongside a tuple of the current error - messages. + messages. """ # instrument returns exponential numbers so converting to float before int error_code = int(float(self.query("print(status.standard.event)"))) diff --git a/src/tm_devices/drivers/device.py b/src/tm_devices/drivers/device.py index 9fe94c72..71658a81 100644 --- a/src/tm_devices/drivers/device.py +++ b/src/tm_devices/drivers/device.py @@ -374,7 +374,7 @@ def check_network_connection(self) -> Tuple[bool, str]: Returns: A tuple containing a boolean indicating if there is a network connection and - a string with the result of the ping command. + a string with the result of the ping command. """ return check_network_connection(self._name_and_alias, self.ip_address) @@ -425,7 +425,7 @@ def get_errors(self) -> Tuple[int, Tuple[str, ...]]: Returns: A tuple containing the current error code alongside a tuple of the current error - messages. + messages. """ return self._get_errors() @@ -438,7 +438,7 @@ def has_errors(self) -> bool: Returns: A boolean indicating if any errors were found in the device. True means there were - errors, False means no errors were found. + errors, False means no errors were found. """ return bool(self.get_errors()[0]) diff --git a/src/tm_devices/drivers/digital_multimeters/dmm6500.py b/src/tm_devices/drivers/digital_multimeters/dmm6500.py index 16f1acef..4338ad2e 100644 --- a/src/tm_devices/drivers/digital_multimeters/dmm6500.py +++ b/src/tm_devices/drivers/digital_multimeters/dmm6500.py @@ -48,7 +48,7 @@ def _get_errors(self) -> Tuple[int, Tuple[str, ...]]: Returns: A tuple containing the current error code alongside a tuple of the current error - messages. + messages. """ # instrument returns exponential numbers so converting to float before int error_code = int(float(self.query("print(status.standard.event)"))) diff --git a/src/tm_devices/drivers/digital_multimeters/dmm75xx/dmm75xx.py b/src/tm_devices/drivers/digital_multimeters/dmm75xx/dmm75xx.py index 9d250751..f1cdb0ca 100644 --- a/src/tm_devices/drivers/digital_multimeters/dmm75xx/dmm75xx.py +++ b/src/tm_devices/drivers/digital_multimeters/dmm75xx/dmm75xx.py @@ -48,7 +48,7 @@ def _get_errors(self) -> Tuple[int, Tuple[str, ...]]: Returns: A tuple containing the current error code alongside a tuple of the current error - messages. + messages. """ # instrument returns exponential numbers so converting to float before int error_code = int(float(self.query("print(status.standard.event)"))) diff --git a/src/tm_devices/drivers/margin_testers/margin_tester.py b/src/tm_devices/drivers/margin_testers/margin_tester.py index c4231705..bb64b562 100644 --- a/src/tm_devices/drivers/margin_testers/margin_tester.py +++ b/src/tm_devices/drivers/margin_testers/margin_tester.py @@ -144,7 +144,7 @@ def _get_errors(self) -> Tuple[int, Tuple[str, ...]]: Returns: A tuple containing the current error code alongside a tuple of the current error - messages. + messages. """ return 0, () diff --git a/src/tm_devices/drivers/source_measure_units/smu24xx/smu24xx_interactive.py b/src/tm_devices/drivers/source_measure_units/smu24xx/smu24xx_interactive.py index a4e58c1b..dd5eb015 100644 --- a/src/tm_devices/drivers/source_measure_units/smu24xx/smu24xx_interactive.py +++ b/src/tm_devices/drivers/source_measure_units/smu24xx/smu24xx_interactive.py @@ -54,7 +54,7 @@ def _get_errors(self) -> Tuple[int, Tuple[str, ...]]: Returns: A tuple containing the current error code alongside a tuple of the current error - messages. + messages. """ # instrument returns exponential numbers so converting to float before int error_code = int(float(self.query("print(status.standard.event)"))) diff --git a/src/tm_devices/helpers/__init__.py b/src/tm_devices/helpers/__init__.py index dc64d8de..e733ae69 100644 --- a/src/tm_devices/helpers/__init__.py +++ b/src/tm_devices/helpers/__init__.py @@ -36,7 +36,7 @@ register_additional_usbtmc_mapping, sanitize_enum, ) -from tm_devices.helpers.logging import configure_logging +from tm_devices.helpers.logging import configure_logging, LoggingLevels from tm_devices.helpers.read_only_cached_property import ReadOnlyCachedProperty from tm_devices.helpers.singleton_metaclass import Singleton from tm_devices.helpers.standalone_functions import validate_address @@ -59,6 +59,7 @@ "get_model_series", "get_version", "get_visa_backend", + "LoggingLevels", "PACKAGE_NAME", "ping_address", "PYVISA_PY_BACKEND", diff --git a/src/tm_devices/helpers/constants_and_dataclasses.py b/src/tm_devices/helpers/constants_and_dataclasses.py index 703a25f4..52feed37 100644 --- a/src/tm_devices/helpers/constants_and_dataclasses.py +++ b/src/tm_devices/helpers/constants_and_dataclasses.py @@ -18,6 +18,7 @@ LoadImpedanceAFG, SupportedModels, ) +from tm_devices.helpers.logging import LoggingLevels from tm_devices.helpers.standalone_functions import validate_address @@ -449,6 +450,67 @@ class DMConfigOptions(AsDictionaryMixin): """ check_for_updates: Optional[bool] = None """A flag indicating if a check for updates for the package should be performed on creation of the DeviceManager.""" # noqa: E501 + log_console_level: Optional[str] = None + """The logging level to set for the console. + + One of `["DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL", "NONE"]`, see the + [`LoggingLevels`][tm_devices.helpers.logging.LoggingLevels] enum for details. + Defaults to `"INFO"`. See the + [`configure_logging()`][tm_devices.helpers.logging.configure_logging] function for more + information. + """ + log_file_level: Optional[str] = None + """The logging level to set for the log file. + + One of `["DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL", "NONE"]`, see the + [`LoggingLevels`][tm_devices.helpers.logging.LoggingLevels] enum for details. + Defaults to `"DEBUG"`. See the + [`configure_logging()`][tm_devices.helpers.logging.configure_logging] function for more + information. + """ + log_file_directory: Optional[str] = None + """The directory to save log files to. + + Defaults to "./logs". See the + [`configure_logging()`][tm_devices.helpers.logging.configure_logging] function for more + information and default values. + """ + log_file_name: Optional[str] = None + """The name of the log file to save the logs to. + + Defaults to a timestamped filename with the .log extension. See the + [`configure_logging()`][tm_devices.helpers.logging.configure_logging] function for more + information and default values. + """ + log_colored_output: Optional[bool] = None + """Whether to use colored output from the `colorlog` package for the console. + + Defaults to False. See the [`configure_logging()`][tm_devices.helpers.logging.configure_logging] + function for more information and default values. + """ + log_pyvisa_messages: Optional[bool] = None + """Whether to include logs from the `pyvisa` package in the log file. + + Defaults to False. See the [`configure_logging()`][tm_devices.helpers.logging.configure_logging] + function for more information and default values. + """ + + def __post_init__(self) -> None: + """Validate data after creation. + + Raises: + ValueError: Indicates a bad input value. + """ + for attribute in ("log_console_level", "log_file_level"): + try: + if (value := getattr(self, attribute)) is not None: + _ = LoggingLevels(value) + except ValueError as error: # noqa: PERF203 + msg = ( + f'Invalid value for {attribute}: "{getattr(self, attribute)}". ' + f"Valid values are {LoggingLevels.list_values()}" + ) + raise ValueError(msg) from error def __str__(self) -> str: """Complete config entry line for an environment variable.""" diff --git a/src/tm_devices/helpers/enums.py b/src/tm_devices/helpers/enums.py index 19102948..c403b9b5 100644 --- a/src/tm_devices/helpers/enums.py +++ b/src/tm_devices/helpers/enums.py @@ -26,23 +26,6 @@ def list_values(cls) -> List[str]: return [enum_entry.value for enum_entry in cls] -class LoggingLevels(CustomStrEnum): - """A class holding the valid logging levels supported.""" - - DEBUG = "DEBUG" - """An enum member representing the DEBUG logging level.""" - INFO = "INFO" - """An enum member representing the INFO logging level.""" - WARNING = "WARNING" - """An enum member representing the WARNING logging level.""" - ERROR = "ERROR" - """An enum member representing the ERROR logging level.""" - CRITICAL = "CRITICAL" - """An enum member representing the CRITICAL logging level.""" - NONE = "NONE" - """An enum member indicating no logging messages should be captured.""" - - class ConfigFileType(CustomStrEnum): """Class holding valid config file extensions.""" diff --git a/src/tm_devices/helpers/functions.py b/src/tm_devices/helpers/functions.py index a461c5b9..36cfc3ed 100644 --- a/src/tm_devices/helpers/functions.py +++ b/src/tm_devices/helpers/functions.py @@ -223,7 +223,7 @@ def check_network_connection(device_name: str, ip_address: str) -> Tuple[bool, s Returns: A boolean indicating if there is a network connection and - a string with the result of the ping command. + a string with the result of the ping command. """ _logger.debug("(%s) ping >> %s", device_name, ip_address) ping_result = ping_address(ip_address) diff --git a/src/tm_devices/helpers/logging.py b/src/tm_devices/helpers/logging.py index 69e73659..26e7c431 100644 --- a/src/tm_devices/helpers/logging.py +++ b/src/tm_devices/helpers/logging.py @@ -2,19 +2,19 @@ import importlib.metadata import logging +import os import sys import time from pathlib import Path -from typing import Optional +from typing import Optional, Union import colorlog import pyvisa from tzlocal import get_localzone # pyright: ignore[reportUnknownVariableType] -from tm_devices.helpers.constants_and_dataclasses import PACKAGE_NAME -from tm_devices.helpers.enums import LoggingLevels +from tm_devices.helpers.enums import CustomStrEnum _logger_initialized = False @@ -27,46 +27,73 @@ def format(self, record: logging.LogRecord) -> str: return super().format(record) +class LoggingLevels(CustomStrEnum): + """A class holding the valid logging levels supported.""" + + DEBUG = "DEBUG" + """An enum member representing the DEBUG logging level.""" + INFO = "INFO" + """An enum member representing the INFO logging level.""" + WARNING = "WARNING" + """An enum member representing the WARNING logging level.""" + ERROR = "ERROR" + """An enum member representing the ERROR logging level.""" + CRITICAL = "CRITICAL" + """An enum member representing the CRITICAL logging level.""" + NONE = "NONE" + """An enum member indicating no logging messages should be captured.""" + + def configure_logging( *, - console_logging_level: LoggingLevels = LoggingLevels.INFO, - file_logging_level: LoggingLevels = LoggingLevels.DEBUG, - log_file_directory: Optional[Path] = None, + log_console_level: Union[str, LoggingLevels] = LoggingLevels.INFO, + log_file_level: Union[str, LoggingLevels] = LoggingLevels.DEBUG, + log_file_directory: Optional[Union[str, os.PathLike[str], Path]] = None, log_file_name: Optional[str] = None, - colored_output: bool = False, - include_pyvisa_logs: bool = True, + log_colored_output: bool = False, + log_pyvisa_messages: bool = False, ) -> logging.Logger: # pragma: no cover """Configure the logging for this package. !!! note After this function is called once, if it is called again, it will not perform any - additional configuration. It will simply return the base logger for the package. + additional configuration. It will simply return the base logger for the package. This means + that if logging is configured explicitly in Python code, then any configuration options set + in the config file or environment variables will be ignored. Args: - console_logging_level: The logging level to set for the console. Defaults to INFO. Set to - [`LoggingLevels.NONE`][tm_devices.helpers.enums.LoggingLevels.NONE] to disable all + log_console_level: The logging level to set for the console. Defaults to INFO. Set to + [`LoggingLevels.NONE`][tm_devices.helpers.logging.LoggingLevels.NONE] to disable all console logging/printouts except for certain warnings and exceptions. - file_logging_level: The logging level to set for the file. Defaults to DEBUG. Set to - [`LoggingLevels.NONE`][tm_devices.helpers.enums.LoggingLevels.NONE] to disable logging + log_file_level: The logging level to set for the file. Defaults to DEBUG. Set to + [`LoggingLevels.NONE`][tm_devices.helpers.logging.LoggingLevels.NONE] to disable logging to a file entirely. - log_file_directory: The directory to save the log file to. Defaults to "./logs" in the + log_file_directory: The directory to save log files to. Defaults to "./logs" in the current working directory. - log_file_name: The name of the log file to save the logs to. Defaults to a timestamped name. - colored_output: Whether to use colored output from the `colorlog` package for the console. - Defaults to False. - include_pyvisa_logs: Whether to include logs from the `pyvisa` package in the log file. The - logging level will match the `file_logging_level`. Defaults to True. + log_file_name: The name of the log file to save the logs to. Defaults to a timestamped name + with the .log extension. + log_colored_output: Whether to use colored output from the `colorlog` package for the + console. Defaults to False. + log_pyvisa_messages: Whether to include logs from the `pyvisa` package in the log file. The + logging level will match the `file_logging_level`. Defaults to False. Returns: The base logger for the package, this base logger can also be accessed using - `logging.getLogger(tm_devices.PACKAGE_NAME)`. + `logging.getLogger(tm_devices.PACKAGE_NAME)`. """ + from tm_devices.helpers.constants_and_dataclasses import ( # pylint: disable=import-outside-toplevel + PACKAGE_NAME, + ) + global _logger_initialized # noqa: PLW0603 _logger: logging.Logger = logging.getLogger(PACKAGE_NAME) if _logger_initialized: # If the logger was previously initialized, just return it return _logger + # Convert object types into enum values + log_console_level = LoggingLevels(log_console_level) + log_file_level = LoggingLevels(log_file_level) # Set the logger level to the lowest level, the handlers will filter out specific levels # based on user configuration _logger.setLevel(logging.DEBUG) @@ -80,30 +107,30 @@ def configure_logging( log_file_directory = Path("./logs") if not log_file_name: log_file_name = f"{PACKAGE_NAME}_{time.strftime('%m-%d-%Y_%H-%M-%S', time.localtime())}.log" - log_filepath = log_file_directory / log_file_name + log_filepath = Path(log_file_directory) / log_file_name - if file_logging_level != LoggingLevels.NONE: + if log_file_level != LoggingLevels.NONE: # Set up logger for tm_devices log_filepath.parent.mkdir(parents=True, exist_ok=True) file_handler = logging.FileHandler(log_filepath, mode="w", encoding="utf-8") file_formatter = _CustomFormatter(logging_file_format_string) file_formatter.default_msec_format = "%s.%06d" # Use 6 digits of precision for milliseconds - file_handler.setLevel(getattr(logging, file_logging_level.value)) + file_handler.setLevel(getattr(logging, log_file_level.value)) file_handler.setFormatter(file_formatter) _logger.addHandler(file_handler) - if include_pyvisa_logs: + if log_pyvisa_messages: # Hook into pyvisa's logging to save it into the same file - pyvisa.logger.setLevel(getattr(logging, file_logging_level.value)) + pyvisa.logger.setLevel(getattr(logging, log_file_level.value)) pyvisa.logger.addHandler(file_handler) # Log a few things to just the file _logger.debug("timezone==%s", get_localzone()) # pyright: ignore[reportUnknownArgumentType] _logger.debug("%s==%s", PACKAGE_NAME, importlib.metadata.version(PACKAGE_NAME)) - if console_logging_level != LoggingLevels.NONE: - if colored_output: + if log_console_level != LoggingLevels.NONE: + if log_colored_output: console_handler = colorlog.StreamHandler(stream=sys.stdout) console_formatter = colorlog.ColoredFormatter( "%(log_color)s" + logging_console_format_string, @@ -116,7 +143,7 @@ def configure_logging( "%s.%06d" # Use 6 digits of precision for milliseconds ) - console_handler.setLevel(getattr(logging, console_logging_level.value)) + console_handler.setLevel(getattr(logging, log_console_level.value)) console_handler.setFormatter(console_formatter) _logger.addHandler(console_handler) diff --git a/tests/conftest.py b/tests/conftest.py index e1f2be20..4f4b3606 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -35,10 +35,10 @@ def emit(self, record: logging.LogRecord) -> None: _logger = configure_logging( - console_logging_level=LoggingLevels.NONE, - file_logging_level=LoggingLevels.DEBUG, - log_file_name="unit_test.log", + log_console_level=LoggingLevels.NONE, + log_file_level=LoggingLevels.DEBUG, log_file_directory=Path(__file__).parent / "logs", + log_file_name="unit_test.log", ) _unit_test_console_handler = _DynamicStreamHandler(stream=sys.stdout) _unit_test_console_handler.setLevel(logging.DEBUG) diff --git a/tests/samples/sample_devices.toml b/tests/samples/sample_devices.toml index 7d931743..400a2784 100644 --- a/tests/samples/sample_devices.toml +++ b/tests/samples/sample_devices.toml @@ -29,6 +29,7 @@ end_input = "none" [options] default_visa_timeout = 1000 +log_file_level = "WARNING" setup_cleanup = false standalone = true teardown_cleanup = false diff --git a/tests/samples/sample_devices.yaml b/tests/samples/sample_devices.yaml index 9082a5fd..400ec99e 100644 --- a/tests/samples/sample_devices.yaml +++ b/tests/samples/sample_devices.yaml @@ -22,6 +22,7 @@ devices: end_input: none options: default_visa_timeout: 1000 + log_file_level: WARNING setup_cleanup: false standalone: true teardown_cleanup: false diff --git a/tests/test_config_parser.py b/tests/test_config_parser.py index dc00091e..4b72db5b 100644 --- a/tests/test_config_parser.py +++ b/tests/test_config_parser.py @@ -32,7 +32,7 @@ def test_nested_config_prefix_mapping() -> None: def test_environment_variable_config(capsys: pytest.CaptureFixture[str]) -> None: """Test the environment variable config method.""" - options = ["STANDALONE", "DEFAULT_VISA_TIMEOUT=10000"] + options = ["STANDALONE", "DEFAULT_VISA_TIMEOUT=10000", "LOG_FILE_LEVEL=NONE"] expected_device_string = ( "address=MSO54-123456,connection_type=TCPIP,device_type=SCOPE,lan_device_name=hislip0" ) @@ -54,6 +54,7 @@ def test_environment_variable_config(capsys: pytest.CaptureFixture[str]) -> None assert str(expected_device) == expected_device_string assert not config.options.teardown_cleanup assert config.options.standalone + assert config.options.log_file_level == "NONE" assert config.options.default_visa_timeout == 10000 assert ( config.devices == expected_entry @@ -61,7 +62,7 @@ def test_environment_variable_config(capsys: pytest.CaptureFixture[str]) -> None # test that the config string representation looks like env declaration expected_entry_string = ( - f"TM_OPTIONS=DEFAULT_VISA_TIMEOUT=10000,STANDALONE\n" + f"TM_OPTIONS=DEFAULT_VISA_TIMEOUT=10000,LOG_FILE_LEVEL=NONE,STANDALONE\n" f"TM_DEVICES=~~~{expected_device_string}~~~" ) print(config) # noqa: T201 @@ -101,7 +102,7 @@ def test_environment_variable_config(capsys: pytest.CaptureFixture[str]) -> None config.devices == expected_entry ), f"\nDevice dictionaries don't match:\n{expected_entry}\n{config.devices}" expected_entry_string = ( - f"TM_OPTIONS=DEFAULT_VISA_TIMEOUT=10000,STANDALONE\n" + f"TM_OPTIONS=DEFAULT_VISA_TIMEOUT=10000,LOG_FILE_LEVEL=NONE,STANDALONE\n" f"TM_DEVICES=~~~{expected_device_string}~~~" ) print(config) # noqa: T201 @@ -149,6 +150,7 @@ def test_file_config_default_path() -> None: verbose_mode: false verbose_visa: false default_visa_timeout: 10000 + log_console_level: DEBUG """ expected_options = DMConfigOptions( standalone=False, @@ -157,6 +159,7 @@ def test_file_config_default_path() -> None: verbose_mode=False, verbose_visa=False, default_visa_timeout=10000, + log_console_level="DEBUG", ) expected_devices = { "SCOPE 1": DeviceConfigEntry( @@ -214,6 +217,7 @@ def test_file_config_non_default_path( verbose_mode=False, verbose_visa=False, default_visa_timeout=1000, + log_file_level="WARNING", ) expected_devices = { "SCOPE 1": DeviceConfigEntry( @@ -370,6 +374,11 @@ def test_file_config_non_default_path( {"TM_DEVICES": "device_type=MT,connection_type=REST_API,address=localhost"}, ValueError, ), + # Test invalid configuration options + ( + {"TM_OPTIONS": "STANDALONE,LOG_FILE_LEVEL=INVALID"}, + ValueError, + ), ], ) def test_invalid_config_creation( diff --git a/tests/validate_device_manager_delete.py b/tests/validate_device_manager_delete.py index 9c03c7b9..ff3260eb 100644 --- a/tests/validate_device_manager_delete.py +++ b/tests/validate_device_manager_delete.py @@ -14,7 +14,7 @@ from tm_devices import configure_logging, DeviceManager, LoggingLevels, PYVISA_PY_BACKEND from tm_devices.helpers import DMConfigOptions -configure_logging(console_logging_level=LoggingLevels.DEBUG) +configure_logging(log_console_level=LoggingLevels.DEBUG) @mock.patch("socket.gethostbyname", mock.MagicMock(side_effect=mock_gethostbyname)) From 990435ea63f129416c14f9d5377606dc5a86d0eb Mon Sep 17 00:00:00 2001 From: "Felt, Nicholas" Date: Mon, 11 Nov 2024 20:17:55 -0800 Subject: [PATCH 07/11] refactor: Update code based on Pull Request build failures --- pyproject.toml | 3 ++- .../drivers/margin_testers/margin_tester.py | 8 ++++-- .../drivers/scopes/tekscope/tekscope.py | 26 +++++++++++-------- src/tm_devices/helpers/logging.py | 15 ++++++++--- tests/conftest.py | 2 +- tests/test_all_device_drivers.py | 6 ++++- 6 files changed, 40 insertions(+), 20 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index c63883b2..f0788f73 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -322,7 +322,7 @@ ignore = [ "ANN401", # Dynamically typed expressions (typing.Any) are disallowed in *args and **kwargs (allowed in this package) "COM812", # Trailing comma missing (allowed in this package) "FA100", # Missing `from __future__ import annotations`, but uses ... (allowed in this package) - "FBT", # flake8-boolean-trap # TODO: remove this exclusion? + "FBT", # flake8-boolean-trap # TODO: enable this "FIX002", # Line contains TO DO (allowed in this package) "ISC001", # single-line-implicit-string-concatenation (handled by formatter) "PYI021", # Docstrings should not be included in stubs (allowed in this package) @@ -443,6 +443,7 @@ setenv = commands_pre = python -m poetry install --no-root --without=main commands = + !tests: python -c "import shutil; shutil.rmtree('dist_{envname}', ignore_errors=True)" !tests: poetry build --output=dist_{envname} !tests: twine check --strict dist_{envname}/* !tests: pre-commit run --all-files diff --git a/src/tm_devices/drivers/margin_testers/margin_tester.py b/src/tm_devices/drivers/margin_testers/margin_tester.py index bb64b562..11fba7ad 100644 --- a/src/tm_devices/drivers/margin_testers/margin_tester.py +++ b/src/tm_devices/drivers/margin_testers/margin_tester.py @@ -1,13 +1,14 @@ """Base Margin Tester device driver module.""" +from __future__ import annotations + import os import time from abc import ABC, abstractmethod from pathlib import Path -from typing import Any, Dict, Mapping, MutableMapping, Tuple, Union +from typing import Any, Dict, Mapping, MutableMapping, Tuple, TYPE_CHECKING, Union -from packaging.version import Version from requests.structures import CaseInsensitiveDict from tm_devices.driver_mixins.device_control import RESTAPIControl @@ -15,6 +16,9 @@ from tm_devices.helpers import DeviceConfigEntry, DeviceTypes from tm_devices.helpers import ReadOnlyCachedProperty as cached_property # noqa: N813 +if TYPE_CHECKING: + from packaging.version import Version + @family_base_class class MarginTester(Device, RESTAPIControl, ABC): diff --git a/src/tm_devices/drivers/scopes/tekscope/tekscope.py b/src/tm_devices/drivers/scopes/tekscope/tekscope.py index 6f03fa24..ecad6895 100644 --- a/src/tm_devices/drivers/scopes/tekscope/tekscope.py +++ b/src/tm_devices/drivers/scopes/tekscope/tekscope.py @@ -1,5 +1,7 @@ """Base TekScope scope device driver module.""" +from __future__ import annotations + import logging # pylint: disable=too-many-lines @@ -12,20 +14,10 @@ from dataclasses import dataclass from pathlib import Path from types import MappingProxyType -from typing import Any, cast, Dict, List, Literal, Optional, Tuple, Type, Union +from typing import Any, cast, Dict, List, Literal, Optional, Tuple, Type, TYPE_CHECKING, Union import pyvisa as visa -from tm_devices.commands import ( - LPD6Commands, - MSO2Commands, - MSO4Commands, - MSO5BCommands, - MSO5Commands, - MSO5LPCommands, - MSO6BCommands, - MSO6Commands, -) from tm_devices.driver_mixins.abstract_device_functionality import ( BaseAFGSourceChannel, BusMixin, @@ -60,6 +52,18 @@ SignalGeneratorOutputPathsBase, ) +if TYPE_CHECKING: + from tm_devices.commands import ( + LPD6Commands, + MSO2Commands, + MSO4Commands, + MSO5BCommands, + MSO5Commands, + MSO5LPCommands, + MSO6BCommands, + MSO6Commands, + ) + _logger: logging.Logger = logging.getLogger(__name__) diff --git a/src/tm_devices/helpers/logging.py b/src/tm_devices/helpers/logging.py index 26e7c431..185466ed 100644 --- a/src/tm_devices/helpers/logging.py +++ b/src/tm_devices/helpers/logging.py @@ -1,21 +1,28 @@ +# pyright: reportUnnecessaryTypeIgnoreComment=none """Helpers for logging.""" +from __future__ import annotations + import importlib.metadata import logging -import os import sys import time from pathlib import Path -from typing import Optional, Union +from typing import Optional, TYPE_CHECKING, Union import colorlog import pyvisa -from tzlocal import get_localzone # pyright: ignore[reportUnknownVariableType] +from tzlocal import ( + get_localzone, # pyright: ignore[reportUnknownVariableType] +) from tm_devices.helpers.enums import CustomStrEnum +if TYPE_CHECKING: + import os + _logger_initialized = False @@ -126,7 +133,7 @@ def configure_logging( pyvisa.logger.addHandler(file_handler) # Log a few things to just the file - _logger.debug("timezone==%s", get_localzone()) # pyright: ignore[reportUnknownArgumentType] + _logger.debug("timezone==%s", get_localzone()) # pyright: ignore[reportUnknownArgumentType,reportUnnecessaryTypeIgnoreComment] _logger.debug("%s==%s", PACKAGE_NAME, importlib.metadata.version(PACKAGE_NAME)) if log_console_level != LoggingLevels.NONE: diff --git a/tests/conftest.py b/tests/conftest.py index 4f4b3606..23d2eeb6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -38,7 +38,7 @@ def emit(self, record: logging.LogRecord) -> None: log_console_level=LoggingLevels.NONE, log_file_level=LoggingLevels.DEBUG, log_file_directory=Path(__file__).parent / "logs", - log_file_name="unit_test.log", + log_file_name=f"unit_test_py{sys.version_info.major}{sys.version_info.minor}.log", ) _unit_test_console_handler = _DynamicStreamHandler(stream=sys.stdout) _unit_test_console_handler.setLevel(logging.DEBUG) diff --git a/tests/test_all_device_drivers.py b/tests/test_all_device_drivers.py index d28afce6..9b3c9c98 100644 --- a/tests/test_all_device_drivers.py +++ b/tests/test_all_device_drivers.py @@ -2,6 +2,7 @@ """Verify that all device drivers and connection types can be used.""" import contextlib +import sys from collections import Counter from pathlib import Path @@ -216,7 +217,10 @@ def test_all_device_drivers() -> None: @pytest.mark.depends(on=["test_device_driver"]) def test_visa_device_command_logging() -> None: """Test the VISA command log file contents.""" - generated_file = Path(__file__).parent / "logs/unit_test_visa_commands_SMU2410-HOSTNAME.log" + generated_file = ( + Path(__file__).parent / f"logs/unit_test_py{sys.version_info.major}{sys.version_info.minor}" + f"_visa_commands_SMU2410-HOSTNAME.log" + ) assert generated_file.exists(), f"File not found: {generated_file}" assert ( generated_file.read_text() From b1bcc7a080134e573d73330fddf99d0350c3f839 Mon Sep 17 00:00:00 2001 From: "Felt, Nicholas" Date: Tue, 12 Nov 2024 09:21:47 -0800 Subject: [PATCH 08/11] test: Add tests for the logging configuration function. --- .gitignore | 1 + src/tm_devices/helpers/logging.py | 6 +-- tests/test_logging.py | 83 ++++++++++++++++++++++++++++++- 3 files changed, 86 insertions(+), 4 deletions(-) diff --git a/.gitignore b/.gitignore index dabcf6f0..9f7fb95e 100644 --- a/.gitignore +++ b/.gitignore @@ -59,6 +59,7 @@ coverage.xml .pytest_cache/ .results*/ tests/samples/generated_stubs/* +tests/generated_**/* tests/verify_devices.yaml prof/ diff --git a/src/tm_devices/helpers/logging.py b/src/tm_devices/helpers/logging.py index 185466ed..c109a7b2 100644 --- a/src/tm_devices/helpers/logging.py +++ b/src/tm_devices/helpers/logging.py @@ -59,7 +59,7 @@ def configure_logging( log_file_name: Optional[str] = None, log_colored_output: bool = False, log_pyvisa_messages: bool = False, -) -> logging.Logger: # pragma: no cover +) -> logging.Logger: """Configure the logging for this package. !!! note @@ -110,9 +110,9 @@ def configure_logging( # the message is even added to the line. logging_file_format_string = "[%(asctime)s] [%(package_name)10s] [%(levelname)8s] %(message)s" logging_console_format_string = "%(asctime)s - %(message)s" - if not log_file_directory: + if not log_file_directory: # pragma: no cover log_file_directory = Path("./logs") - if not log_file_name: + if not log_file_name: # pragma: no cover log_file_name = f"{PACKAGE_NAME}_{time.strftime('%m-%d-%Y_%H-%M-%S', time.localtime())}.log" log_filepath = Path(log_file_directory) / log_file_name diff --git a/tests/test_logging.py b/tests/test_logging.py index 176a026f..bb9712e0 100644 --- a/tests/test_logging.py +++ b/tests/test_logging.py @@ -2,12 +2,20 @@ import contextlib import logging +import shutil +import sys +from pathlib import Path from typing import Generator, TYPE_CHECKING +import colorlog import pytest +import pyvisa -from tm_devices import DeviceManager, PACKAGE_NAME +import tm_devices + +from tm_devices import configure_logging, DeviceManager, LoggingLevels, PACKAGE_NAME +from tm_devices.helpers import logging as tm_devices_logging if TYPE_CHECKING: from tm_devices.drivers import MSO2 @@ -37,3 +45,76 @@ def test_visa_command_logging_edge_cases( """Test VISA command logging edge cases.""" scope: MSO2 = device_manager.add_scope("MSO22-HOSTNAME") assert scope.model == "MSO22" + + +def test_logging_singleton() -> None: + """Verify the singleton behavior of the logging configuration function.""" + package_logger = logging.getLogger(PACKAGE_NAME) + logger_handlers_copy = package_logger.handlers.copy() + assert len(logger_handlers_copy) == 3 + logger = configure_logging() + assert len(logger.handlers) == 3 + assert logger.handlers == logger_handlers_copy + + +@pytest.fixture(name="reset_package_logger") +def _reset_package_logger() -> Generator[None, None, None]: # pyright: ignore[reportUnusedFunction] + """Reset the package logger.""" + logger = logging.getLogger(PACKAGE_NAME) + handlers_copy = logger.handlers.copy() + for handler in handlers_copy: + logger.removeHandler(handler) + tm_devices_logging._logger_initialized = False # noqa: SLF001 # pyright: ignore[reportPrivateUsage] + yield + # Reset the handlers back to what they were + for handler in logger.handlers.copy(): + logger.removeHandler(handler) + for handler in handlers_copy: + logger.addHandler(handler) + + +def test_configure_logger_full(reset_package_logger: None) -> None: # noqa: ARG001 + """Test the configuration function with all types of logs.""" + log_dir = ( + Path(__file__).parent / f"generated_logs_py{sys.version_info.major}{sys.version_info.minor}" + ) + log_name = "custom_log.log" + shutil.rmtree(log_dir, ignore_errors=True) + + assert not any(isinstance(handler, logging.FileHandler) for handler in pyvisa.logger.handlers) + assert len(logging.getLogger(PACKAGE_NAME).handlers) == 0 # pylint: disable=use-implicit-booleaness-not-comparison-to-zero + logger = configure_logging( + log_console_level="DEBUG", + log_file_level="DEBUG", + log_file_directory=log_dir, + log_file_name=log_name, + log_colored_output=False, + log_pyvisa_messages=True, + ) + assert len(logger.handlers) == 3 + assert any(isinstance(handler, logging.FileHandler) for handler in pyvisa.logger.handlers) + log_contents = (log_dir / log_name).read_text().split("\n") + assert len(log_contents) == 3 + assert f"] [{PACKAGE_NAME}] [ DEBUG] timezone==" in log_contents[0] + assert log_contents[1].endswith( + f"] [{PACKAGE_NAME}] [ DEBUG] {PACKAGE_NAME}=={tm_devices.__version__}" + ) + assert [type(x) for x in logger.handlers] == [ + logging.NullHandler, + logging.FileHandler, + logging.StreamHandler, + ] + + +def test_configure_logger_no_file(reset_package_logger: None) -> None: # noqa: ARG001 + """Test the configuration function with no file logging.""" + assert len(logging.getLogger(PACKAGE_NAME).handlers) == 0 # pylint: disable=use-implicit-booleaness-not-comparison-to-zero + logger = configure_logging( + log_console_level="DEBUG", + log_file_level=LoggingLevels.NONE, + log_colored_output=True, + log_pyvisa_messages=False, + ) + assert len(logger.handlers) == 2 + assert [type(x) for x in logger.handlers] == [logging.NullHandler, colorlog.StreamHandler] + assert isinstance(logger.handlers[1].formatter, colorlog.ColoredFormatter) From 19a267e183afb294a8dae71cda6d03a3eb6e9ff8 Mon Sep 17 00:00:00 2001 From: "Felt, Nicholas" Date: Tue, 12 Nov 2024 10:54:09 -0800 Subject: [PATCH 09/11] docs: Update IP addresses used in examples --- README.md | 4 ++-- docs/basic_usage.md | 2 +- examples/miscellaneous/customize_logging.py | 2 +- examples/miscellaneous/register_dm_atexit.py | 2 +- examples/scopes/tekscope/basic_save_recall.py | 2 +- examples/scopes/tekscope/generate_internal_afg_signal.py | 2 +- examples/scopes/tekscope/save_screenshot.py | 2 +- .../source_measure_units/2400/smu_2450_leakage_current.py | 2 +- .../2400/smu_2450_measuring_lowr_devices.py | 2 +- .../2400/smu_2450_rechargeable_battery.py | 2 +- examples/source_measure_units/2400/smu_2450_solar_cell.py | 2 +- 11 files changed, 12 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 6c7c397d..7b1e995c 100644 --- a/README.md +++ b/README.md @@ -50,7 +50,7 @@ pip install tm_devices ```console $ list-visa-resources [ - "TCPIP0::192.168.0.100::inst0::INSTR", + "TCPIP0::192.168.0.1::inst0::INSTR", "ASRL4::INSTR" ] ``` @@ -61,7 +61,7 @@ $ list-visa-resources from tm_devices import DeviceManager with DeviceManager() as device_manager: - scope = device_manager.add_scope("192.168.0.100") + scope = device_manager.add_scope("192.168.0.1") scope.query("*IDN?") print(scope) ``` diff --git a/docs/basic_usage.md b/docs/basic_usage.md index 1c448c5d..59c7c205 100644 --- a/docs/basic_usage.md +++ b/docs/basic_usage.md @@ -10,7 +10,7 @@ This will print the available VISA devices to the console when run from a shell ```console $ list-visa-resources [ - "TCPIP0::192.168.0.100::inst0::INSTR", + "TCPIP0::192.168.0.1::inst0::INSTR", "ASRL4::INSTR" ] ``` diff --git a/examples/miscellaneous/customize_logging.py b/examples/miscellaneous/customize_logging.py index 618a1b0d..5f6cc3b9 100644 --- a/examples/miscellaneous/customize_logging.py +++ b/examples/miscellaneous/customize_logging.py @@ -14,7 +14,7 @@ ) with DeviceManager(verbose=False) as dm: - scope: MSO6B = dm.add_scope("192.168.1.5") + scope: MSO6B = dm.add_scope("192.168.0.1") scope.curve_query(1) scope.check_port_connection(4000) scope.check_network_connection() diff --git a/examples/miscellaneous/register_dm_atexit.py b/examples/miscellaneous/register_dm_atexit.py index 7207c538..e0ba70aa 100644 --- a/examples/miscellaneous/register_dm_atexit.py +++ b/examples/miscellaneous/register_dm_atexit.py @@ -12,7 +12,7 @@ atexit.register(dm.close) # Add a device -scope: MSO6B = dm.add_scope("192.168.1.102") +scope: MSO6B = dm.add_scope("192.168.0.1") # Use the device print(scope) diff --git a/examples/scopes/tekscope/basic_save_recall.py b/examples/scopes/tekscope/basic_save_recall.py index 33ac6b1a..03c6ca3e 100644 --- a/examples/scopes/tekscope/basic_save_recall.py +++ b/examples/scopes/tekscope/basic_save_recall.py @@ -5,7 +5,7 @@ with DeviceManager(verbose=True) as dm: # Get a scope - scope: MSO6B = dm.add_scope("192.168.1.177") + scope: MSO6B = dm.add_scope("192.168.0.1") # Send some commands scope.add_new_math("MATH1", "CH1") # add MATH1 to CH1 diff --git a/examples/scopes/tekscope/generate_internal_afg_signal.py b/examples/scopes/tekscope/generate_internal_afg_signal.py index e66eca9b..cf0a05f7 100644 --- a/examples/scopes/tekscope/generate_internal_afg_signal.py +++ b/examples/scopes/tekscope/generate_internal_afg_signal.py @@ -5,7 +5,7 @@ with DeviceManager(verbose=True) as dm: # Create a connection to the scope and indicate that it is a MSO5 scope for type hinting - scope: MSO5 = dm.add_scope("192.168.1.102") + scope: MSO5 = dm.add_scope("192.168.0.1") # Generate the signal using individual PI commands. scope.commands.afg.frequency.write(10e6) # set frequency diff --git a/examples/scopes/tekscope/save_screenshot.py b/examples/scopes/tekscope/save_screenshot.py index 7146ff3e..3503c3ba 100644 --- a/examples/scopes/tekscope/save_screenshot.py +++ b/examples/scopes/tekscope/save_screenshot.py @@ -5,7 +5,7 @@ with DeviceManager(verbose=True) as dm: # Add a scope - scope: MSO6B = dm.add_scope("192.168.1.5") + scope: MSO6B = dm.add_scope("192.168.0.1") # Send some commands scope.add_new_math("MATH1", "CH1") # add MATH1 to CH1 diff --git a/examples/source_measure_units/2400/smu_2450_leakage_current.py b/examples/source_measure_units/2400/smu_2450_leakage_current.py index 33cf1288..243e2918 100644 --- a/examples/source_measure_units/2400/smu_2450_leakage_current.py +++ b/examples/source_measure_units/2400/smu_2450_leakage_current.py @@ -13,7 +13,7 @@ from tm_devices.drivers import SMU2450 with DeviceManager(verbose=False) as device_manager: - inst: SMU2450 = device_manager.add_smu("192.168.1.4", alias="my2450") + inst: SMU2450 = device_manager.add_smu("192.168.0.1", alias="my2450") # Reset the instrument, which also clears the buffer. inst.commands.reset() diff --git a/examples/source_measure_units/2400/smu_2450_measuring_lowr_devices.py b/examples/source_measure_units/2400/smu_2450_measuring_lowr_devices.py index aa930091..08c32e3c 100644 --- a/examples/source_measure_units/2400/smu_2450_measuring_lowr_devices.py +++ b/examples/source_measure_units/2400/smu_2450_measuring_lowr_devices.py @@ -21,7 +21,7 @@ with DeviceManager() as device_manager: print(device_manager.get_available_devices()) - inst: SMU2450 = device_manager.add_smu("192.168.4.74", alias="my2450") + inst: SMU2450 = device_manager.add_smu("192.168.0.1", alias="my2450") # Configure the Simple Loop trigger model template to make 100 readings. inst.commands.trigger.model.load_simple_loop(100) diff --git a/examples/source_measure_units/2400/smu_2450_rechargeable_battery.py b/examples/source_measure_units/2400/smu_2450_rechargeable_battery.py index 212b9142..7cc0983e 100644 --- a/examples/source_measure_units/2400/smu_2450_rechargeable_battery.py +++ b/examples/source_measure_units/2400/smu_2450_rechargeable_battery.py @@ -19,7 +19,7 @@ with DeviceManager() as device_manager: print(device_manager.get_available_devices()) - inst: SMU2450 = device_manager.add_smu("192.168.4.74", alias="my2450") + inst: SMU2450 = device_manager.add_smu("192.168.0.1", alias="my2450") # Clear the buffer. inst.commands.buffer_var["defbuffer1"].clear() diff --git a/examples/source_measure_units/2400/smu_2450_solar_cell.py b/examples/source_measure_units/2400/smu_2450_solar_cell.py index 56dba26c..05589d3e 100644 --- a/examples/source_measure_units/2400/smu_2450_solar_cell.py +++ b/examples/source_measure_units/2400/smu_2450_solar_cell.py @@ -15,7 +15,7 @@ with DeviceManager() as device_manager: print(device_manager.get_available_devices()) - inst: SMU2450 = device_manager.add_smu("192.168.4.74", alias="my2450") + inst: SMU2450 = device_manager.add_smu("192.168.0.1", alias="my2450") # Define the number of points in the sweep. POINTS = 56 From 3de696b0f1369780ccda0f306f7d4e098baaedcc Mon Sep 17 00:00:00 2001 From: "Felt, Nicholas" Date: Tue, 12 Nov 2024 11:35:08 -0800 Subject: [PATCH 10/11] test: Update some tests to be less flaky --- tests/test_scopes.py | 5 ++--- tests/test_smu.py | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/test_scopes.py b/tests/test_scopes.py index 40b29941..c4201efa 100644 --- a/tests/test_scopes.py +++ b/tests/test_scopes.py @@ -401,9 +401,8 @@ def test_tekscope70k( assert not scope.wait_for_network_connection( wait_time=0.05, sleep_seconds=1, accept_immediate_connection=True ) - assert caplog.records[-1].message == ( - f"Unable to establish a network connection with " - f"{scope.ip_address} after 0.05 seconds" + assert caplog.records[-1].message.startswith( + f"Unable to establish a network connection with {scope.ip_address} after" ) assert caplog.records[-1].levelname == "WARNING" diff --git a/tests/test_smu.py b/tests/test_smu.py index 68d468a1..f875c7ab 100644 --- a/tests/test_smu.py +++ b/tests/test_smu.py @@ -213,9 +213,8 @@ def test_smu( # noqa: PLR0915 assert not smu.wait_for_port_connection( 4000, wait_time=0.05, sleep_seconds=0, accept_immediate_connection=True ) - assert caplog.records[-1].message == ( - f"Unable to establish a connection to port 4000 " - f"on {smu.ip_address} after 0.05 seconds" + assert caplog.records[-1].message.startswith( + f"Unable to establish a connection to port 4000 on {smu.ip_address} after" ) assert caplog.records[-1].levelname == "WARNING" From c7348a53449141f1728acc54c3b078f6e923b464 Mon Sep 17 00:00:00 2001 From: "Felt, Nicholas" Date: Fri, 15 Nov 2024 08:26:31 -0800 Subject: [PATCH 11/11] refactor: Add leading tabs to the standard logging output of the multi-line VISA write. Also updated unit test simulated devices and locked the version of pyright. --- pyproject.toml | 2 +- src/tm_devices/driver_mixins/device_control/pi_control.py | 2 +- tests/samples/tsp_script.tsp | 2 +- tests/sim_devices/smu/smu2601b.yaml | 4 ++-- tests/sim_devices/smu/smu2601b_pulse.yaml | 2 +- tests/sim_devices/smu/smu2602b.yaml | 2 +- tests/sim_devices/smu/smu2604b.yaml | 2 +- tests/sim_devices/smu/smu2606b.yaml | 2 +- tests/sim_devices/smu/smu2611b.yaml | 2 +- tests/sim_devices/smu/smu2612b.yaml | 2 +- tests/sim_devices/smu/smu2614b.yaml | 2 +- tests/sim_devices/smu/smu2634b.yaml | 2 +- tests/sim_devices/smu/smu2635b.yaml | 2 +- tests/sim_devices/smu/smu2636b.yaml | 2 +- tests/sim_devices/smu/smu2651a.yaml | 2 +- tests/sim_devices/smu/smu2657a.yaml | 2 +- 16 files changed, 17 insertions(+), 17 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 2e90b2e6..24e3b86c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -112,7 +112,7 @@ pre-commit = [ {python = "3.8", version = "^3.5"} ] pylint = "3.2.7" -pyright = {extras = ["nodejs"], version = "^1.1.387"} +pyright = {extras = ["nodejs"], version = "1.1.389"} pyroma = "^4.2" tox = "^4.0" tox-gh-actions = "^3.1.0" diff --git a/src/tm_devices/driver_mixins/device_control/pi_control.py b/src/tm_devices/driver_mixins/device_control/pi_control.py index fe4a92ed..b743ad65 100644 --- a/src/tm_devices/driver_mixins/device_control/pi_control.py +++ b/src/tm_devices/driver_mixins/device_control/pi_control.py @@ -847,7 +847,7 @@ def write(self, command: str, opc: bool = False, verbose: bool = True) -> None: logging.INFO if self._verbose and verbose else logging.DEBUG, "(%s) Write >>\n%s", self._name_and_alias, - command, + "\n".join([" " + x for x in command.split("\n")]), ) else: _logger.log( diff --git a/tests/samples/tsp_script.tsp b/tests/samples/tsp_script.tsp index 60da9791..4a51ccc6 100644 --- a/tests/samples/tsp_script.tsp +++ b/tests/samples/tsp_script.tsp @@ -1,2 +1,2 @@ -"""Sample script used in test_smu.py.""" +-- Sample script used in test_smu.py. print("TEK") diff --git a/tests/sim_devices/smu/smu2601b.yaml b/tests/sim_devices/smu/smu2601b.yaml index e08146ff..241c9a55 100644 --- a/tests/sim_devices/smu/smu2601b.yaml +++ b/tests/sim_devices/smu/smu2601b.yaml @@ -7,8 +7,8 @@ devices: r: Keithley Instruments Inc., Model 2601B, 4498311, 3.3.5 - q: print(available(gpib)) r: 'true' - - q: loadscript loadfuncs\n"""Sample script used in test_smu.py."""\nprint("TEK")\nendscript - - q: loadscript tsp_function\n"""Sample script used in test_smu.py."""\nprint("TEK")\nendscript + - q: loadscript loadfuncs\n-- Sample script used in test_smu.py.\nprint("TEK")\nendscript + - q: loadscript tsp_function\n-- Sample script used in test_smu.py.\nprint("TEK")\nendscript - q: loadfuncs.save() - q: loadfuncs() - q: errorqueue.clear() diff --git a/tests/sim_devices/smu/smu2601b_pulse.yaml b/tests/sim_devices/smu/smu2601b_pulse.yaml index 8cfd52d2..9627689c 100644 --- a/tests/sim_devices/smu/smu2601b_pulse.yaml +++ b/tests/sim_devices/smu/smu2601b_pulse.yaml @@ -10,7 +10,7 @@ devices: - q: loadscript tsp_script - q: loadscript tsp_function - q: print("TEK") - - q: '"""Sample script used in test_smu.py."""' + - q: -- Sample script used in test_smu.py. - q: endscript - q: tsp_script.save() - q: tsp_script() diff --git a/tests/sim_devices/smu/smu2602b.yaml b/tests/sim_devices/smu/smu2602b.yaml index 6a85a12d..3f29fc5e 100644 --- a/tests/sim_devices/smu/smu2602b.yaml +++ b/tests/sim_devices/smu/smu2602b.yaml @@ -10,7 +10,7 @@ devices: - q: loadscript tsp_script - q: loadscript tsp_function - q: print("TEK") - - q: '"""Sample script used in test_smu.py."""' + - q: -- Sample script used in test_smu.py. - q: endscript - q: tsp_script.save() - q: tsp_script() diff --git a/tests/sim_devices/smu/smu2604b.yaml b/tests/sim_devices/smu/smu2604b.yaml index df0a1a25..6c167073 100644 --- a/tests/sim_devices/smu/smu2604b.yaml +++ b/tests/sim_devices/smu/smu2604b.yaml @@ -10,7 +10,7 @@ devices: - q: loadscript tsp_script - q: loadscript tsp_function - q: print("TEK") - - q: '"""Sample script used in test_smu.py."""' + - q: -- Sample script used in test_smu.py. - q: endscript - q: tsp_script.save() - q: tsp_script() diff --git a/tests/sim_devices/smu/smu2606b.yaml b/tests/sim_devices/smu/smu2606b.yaml index b7958699..8624bc7c 100644 --- a/tests/sim_devices/smu/smu2606b.yaml +++ b/tests/sim_devices/smu/smu2606b.yaml @@ -10,7 +10,7 @@ devices: - q: loadscript tsp_script - q: loadscript tsp_function - q: print("TEK") - - q: '"""Sample script used in test_smu.py."""' + - q: -- Sample script used in test_smu.py. - q: endscript - q: tsp_script.save() - q: tsp_script() diff --git a/tests/sim_devices/smu/smu2611b.yaml b/tests/sim_devices/smu/smu2611b.yaml index f7e01632..c0e17f00 100644 --- a/tests/sim_devices/smu/smu2611b.yaml +++ b/tests/sim_devices/smu/smu2611b.yaml @@ -10,7 +10,7 @@ devices: - q: loadscript tsp_script - q: loadscript tsp_function - q: print("TEK") - - q: '"""Sample script used in test_smu.py."""' + - q: -- Sample script used in test_smu.py. - q: endscript - q: tsp_script.save() - q: tsp_script() diff --git a/tests/sim_devices/smu/smu2612b.yaml b/tests/sim_devices/smu/smu2612b.yaml index 3355842c..e686bb88 100644 --- a/tests/sim_devices/smu/smu2612b.yaml +++ b/tests/sim_devices/smu/smu2612b.yaml @@ -10,7 +10,7 @@ devices: - q: loadscript tsp_script - q: loadscript tsp_function - q: print("TEK") - - q: '"""Sample script used in test_smu.py."""' + - q: -- Sample script used in test_smu.py. - q: endscript - q: tsp_script.save() - q: tsp_script() diff --git a/tests/sim_devices/smu/smu2614b.yaml b/tests/sim_devices/smu/smu2614b.yaml index a6eda98f..87d56c88 100644 --- a/tests/sim_devices/smu/smu2614b.yaml +++ b/tests/sim_devices/smu/smu2614b.yaml @@ -10,7 +10,7 @@ devices: - q: loadscript tsp_script - q: loadscript tsp_function - q: print("TEK") - - q: '"""Sample script used in test_smu.py."""' + - q: -- Sample script used in test_smu.py. - q: endscript - q: tsp_script.save() - q: tsp_script() diff --git a/tests/sim_devices/smu/smu2634b.yaml b/tests/sim_devices/smu/smu2634b.yaml index b35e8f9c..e66283f6 100644 --- a/tests/sim_devices/smu/smu2634b.yaml +++ b/tests/sim_devices/smu/smu2634b.yaml @@ -10,7 +10,7 @@ devices: - q: loadscript tsp_script - q: loadscript tsp_function - q: print("TEK") - - q: '"""Sample script used in test_smu.py."""' + - q: -- Sample script used in test_smu.py. - q: endscript - q: tsp_script.save() - q: tsp_script() diff --git a/tests/sim_devices/smu/smu2635b.yaml b/tests/sim_devices/smu/smu2635b.yaml index 271b6e3e..7d2db1c5 100644 --- a/tests/sim_devices/smu/smu2635b.yaml +++ b/tests/sim_devices/smu/smu2635b.yaml @@ -10,7 +10,7 @@ devices: - q: loadscript tsp_script - q: loadscript tsp_function - q: print("TEK") - - q: '"""Sample script used in test_smu.py."""' + - q: -- Sample script used in test_smu.py. - q: endscript - q: tsp_script.save() - q: tsp_script() diff --git a/tests/sim_devices/smu/smu2636b.yaml b/tests/sim_devices/smu/smu2636b.yaml index 6fef3be0..c46ac58f 100644 --- a/tests/sim_devices/smu/smu2636b.yaml +++ b/tests/sim_devices/smu/smu2636b.yaml @@ -10,7 +10,7 @@ devices: - q: loadscript tsp_script - q: loadscript tsp_function - q: print("TEK") - - q: '"""Sample script used in test_smu.py."""' + - q: -- Sample script used in test_smu.py. - q: endscript - q: tsp_script.save() - q: tsp_script() diff --git a/tests/sim_devices/smu/smu2651a.yaml b/tests/sim_devices/smu/smu2651a.yaml index f99a2492..58abba85 100644 --- a/tests/sim_devices/smu/smu2651a.yaml +++ b/tests/sim_devices/smu/smu2651a.yaml @@ -10,7 +10,7 @@ devices: - q: loadscript tsp_script - q: loadscript tsp_function - q: print("TEK") - - q: '"""Sample script used in test_smu.py."""' + - q: -- Sample script used in test_smu.py. - q: endscript - q: tsp_script.save() - q: tsp_script() diff --git a/tests/sim_devices/smu/smu2657a.yaml b/tests/sim_devices/smu/smu2657a.yaml index 32550b72..ca306550 100644 --- a/tests/sim_devices/smu/smu2657a.yaml +++ b/tests/sim_devices/smu/smu2657a.yaml @@ -10,7 +10,7 @@ devices: - q: loadscript tsp_script - q: loadscript tsp_function - q: print("TEK") - - q: '"""Sample script used in test_smu.py."""' + - q: -- Sample script used in test_smu.py. - q: endscript - q: tsp_script.save() - q: tsp_script()