Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restore time_sync module on macOS #75

Merged
merged 11 commits into from
Sep 13, 2023
1 change: 0 additions & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ jobs:
python-version: ["3.7", "3.8", "3.9", "3.10", "3.11"]
include:
- python-version: "3.10"
os: ubuntu-20.04
bmerry marked this conversation as resolved.
Show resolved Hide resolved
coverage: "yes"
steps:
- uses: actions/checkout@v3
Expand Down
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ repos:
hooks:
- id: flake8
- repo: https://github.com/pre-commit/mirrors-mypy
rev: 'v0.990'
rev: 'v1.4.1'
hooks:
- id: mypy
# Passing filenames to mypy can do odd things. See
Expand Down
8 changes: 2 additions & 6 deletions src/aiokatcp/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright 2017, 2022 National Research Foundation (SARAO)
# Copyright 2017, 2022, 2023 National Research Foundation (SARAO)
#
# Redistribution and use in source and binary forms, with or without
# modification, are permitted provided that the following conditions are met:
Expand Down Expand Up @@ -37,8 +37,6 @@
__version__ = _katversion.get_version(__path__[0])
# END VERSION CHECK

import sys

from .client import ( # noqa: F401
AbstractSensorWatcher,
Client,
Expand Down Expand Up @@ -70,9 +68,7 @@
SimpleAggregateSensor,
)
from .server import DeviceServer, RequestContext # noqa: F401

if sys.platform == "linux":
from .time_sync import ClockState, TimeSyncUpdater # noqa: F401
from .time_sync import ClockState, TimeSyncUpdater # noqa: F401


def minor_version():
Expand Down
32 changes: 21 additions & 11 deletions src/aiokatcp/adjtimex.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright 2022 National Research Foundation (SARAO)
# Copyright 2022, 2023 National Research Foundation (SARAO)
#
# Redistribution and use in source and binary forms, with or without
# modification, are permitted provided that the following conditions are met:
Expand All @@ -25,10 +25,7 @@
# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

"""Python wrapper for the Linux-specific :func:`adjtimex` system call.

Importing this module on a non-Linux system is likely to fail.
"""
"""Python wrapper for the Linux-specific :func:`adjtimex` system call."""

import ctypes
import os
Expand Down Expand Up @@ -85,13 +82,17 @@


class Timeval(ctypes.Structure):
"""See https://man7.org/linux/man-pages/man3/adjtime.3.html."""

_fields_ = [
("tv_sec", ctypes.c_long),
("tv_usec", ctypes.c_long),
]


class Timex(ctypes.Structure):
"""See https://man7.org/linux/man-pages/man2/adjtimex.2.html."""

_fields_ = [
("modes", ctypes.c_int),
("offset", ctypes.c_long),
Expand All @@ -117,18 +118,27 @@ class Timex(ctypes.Structure):
]


def _no_adjtimex(timex: Timex) -> int:
raise NotImplementedError("System call 'adjtimex' is only available on Linux")


def _errcheck(result, func, args):
if result == -1:
e = ctypes.get_errno()
raise OSError(e, os.strerror(e))
return result


_libc = ctypes.CDLL("libc.so.6", use_errno=True)
adjtimex = _libc.adjtimex
adjtimex.argtypes = [ctypes.POINTER(Timex)]
adjtimex.restype = ctypes.c_int
adjtimex.errcheck = _errcheck # type: ignore
try:
_libc = ctypes.CDLL("libc.so.6", use_errno=True)
except OSError:
adjtimex = _no_adjtimex
else:
_real_adjtimex = _libc.adjtimex
_real_adjtimex.argtypes = [ctypes.POINTER(Timex)]
_real_adjtimex.restype = ctypes.c_int
_real_adjtimex.errcheck = _errcheck
adjtimex = _real_adjtimex


def get_adjtimex() -> Tuple[int, Timex]:
Expand All @@ -142,6 +152,6 @@ def get_adjtimex() -> Tuple[int, Timex]:
Clock information
"""
timex = Timex()
timex.mode = 0
timex.modes = 0
bmerry marked this conversation as resolved.
Show resolved Hide resolved
state = adjtimex(timex)
return state, timex
10 changes: 8 additions & 2 deletions src/aiokatcp/time_sync.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright 2022 National Research Foundation (SARAO)
# Copyright 2022, 2023 National Research Foundation (SARAO)
#
# Redistribution and use in source and binary forms, with or without
# modification, are permitted provided that the following conditions are met:
Expand Down Expand Up @@ -84,7 +84,13 @@ def __init__(self, sensor_map: Mapping[str, Sensor]) -> None:

def update(self) -> None:
"""Update the sensors now."""
state, timex = adjtimex.get_adjtimex()
try:
state, timex = adjtimex.get_adjtimex()
except NotImplementedError:
for sensor in self.sensor_map.values():
sensor.set_value(sensor.value, Sensor.Status.INACTIVE)
return

if timex.status & adjtimex.STA_NANO:
scale = 1e-9
else:
Expand Down
28 changes: 21 additions & 7 deletions tests/test_time_sync.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright 2017, 2022 National Research Foundation (SARAO)
# Copyright 2017, 2022, 2023 National Research Foundation (SARAO)
#
# Redistribution and use in source and binary forms, with or without
# modification, are permitted provided that the following conditions are met:
Expand Down Expand Up @@ -31,9 +31,6 @@

import pytest

if sys.platform != "linux":
pytest.skip("skipping Linux-only tests based on adjtimex", allow_module_level=True)

import aiokatcp.adjtimex
from aiokatcp import Sensor, SensorSet
from aiokatcp.time_sync import ClockState, TimeSyncUpdater
Expand All @@ -53,6 +50,14 @@ def updater(sensors: SensorSet) -> TimeSyncUpdater:
return TimeSyncUpdater({sensor.name[4:]: sensor for sensor in sensors.values()})


@pytest.fixture(params=[False, True])
def pretend_no_adjtimex(mocker, request) -> bool:
"""Optionally replace adjtimex with _no_adjtimex (i.e. pretend it is absent)."""
if request.param:
mocker.patch("aiokatcp.adjtimex.adjtimex", side_effect=aiokatcp.adjtimex._no_adjtimex)
return request.param


@pytest.fixture(params=[False, True])
def mock_adjtimex(mocker, request) -> None:
"""Replace get_adjtimex with a mock version with known values."""
Expand All @@ -62,22 +67,31 @@ def mock_adjtimex(mocker, request) -> None:
timex.maxerror = 4567
timex.time.tv_sec = 1234567890
timex.time.tv_usec = 654321
# Check both microsecond and nanosecond versions of the timex struct
if request.param:
timex.status |= aiokatcp.adjtimex.STA_NANO
timex.time.tv_usec *= 1000
return_value = (aiokatcp.adjtimex.TIME_OK, timex)
mocker.patch("aiokatcp.adjtimex.get_adjtimex", return_value=return_value)


def test_smoke(sensors: SensorSet, updater: TimeSyncUpdater) -> None:
"""Test with real adjtimex, to make sure it interacts cleanly with the kernel."""
def test_smoke(sensors: SensorSet, updater: TimeSyncUpdater, pretend_no_adjtimex) -> None:
"""Test with real adjtimex, to make sure it interacts cleanly with the kernel.

On non-Linux systems, check that the sensors are inactive instead. Also
pretend that we don't have adjtimex just to check both code paths on Linux.
"""
updater.update()
assert isinstance(sensors["ntp.esterror"].value, float)
assert isinstance(sensors["ntp.maxerror"].value, float)
assert isinstance(sensors["ntp.state"].value, ClockState)
if not pretend_no_adjtimex and sys.platform == "linux":
expected_status = Sensor.Status.NOMINAL
else:
expected_status = Sensor.Status.INACTIVE
for sensor in sensors.values():
# Check that it actually got updated
assert sensor.status == Sensor.Status.NOMINAL
assert sensor.status == expected_status


def test_bad_key(sensors) -> None:
Expand Down