Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion backend/core/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,9 @@ class NodeConfig(Base):
tx_lat: Mapped[float | None] = mapped_column(Float, nullable=True)
tx_lon: Mapped[float | None] = mapped_column(Float, nullable=True)
tx_alt_ft: Mapped[float | None] = mapped_column(Float, nullable=True)
tx_callsign: Mapped[str] = mapped_column(String(32))
# Nullable since 1.2.2: an owner who cannot name the illuminator, the same
# case the coordinates above carry.
tx_callsign: Mapped[str | None] = mapped_column(String(32), nullable=True)
fc_hz: Mapped[float] = mapped_column(Float)
fs_hz: Mapped[float] = mapped_column(Float)
# Both nullable, and neither null may be filled in. A null width means the
Expand Down
37 changes: 37 additions & 0 deletions backend/migrations/versions/0009_nullable_tx_callsign.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
"""tx_callsign becomes nullable on node_configs.

Revision ID: 0009
Revises: 0008
"""

import sqlalchemy as sa
from alembic import op

revision = "0009"
down_revision = "0008"
branch_labels = None
depends_on = None

# A downgrade cannot express a null, and code predating 1.2.2 has no
# null-handling for this column, so a rollback across this revision must be
# surfaced to a human rather than served as safe. The same grading, for the same
# reason, as 0005 on the coordinates.
rollback_safety = "destructive"


def upgrade() -> None:
# batch_alter_table because SQLite has no ALTER COLUMN: Alembic copies the
# table with the corrected definition and swaps it in. Existing rows keep the
# names they declared; the table is append-only, so this governs new versions
# only.
with op.batch_alter_table("node_configs") as batch:
batch.alter_column("tx_callsign", existing_type=sa.String(length=32), nullable=True)


def downgrade() -> None:
# Fails, loudly, once any node has registered without a callsign: the table
# copy hits the NOT NULL and leaves the database stamped at 0009. That is the
# honest outcome, since the alternative is inventing an illuminator name that
# nothing downstream could tell from one an owner gave.
with op.batch_alter_table("node_configs") as batch:
batch.alter_column("tx_callsign", existing_type=sa.String(length=32), nullable=False)
11 changes: 10 additions & 1 deletion backend/routes/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,16 @@
# would be 1.3.0. Recorded so the block stays honest with itself, and because
# nothing about the choice is load-bearing — the field is optional either way,
# and no node behaves differently for the number in front of it.
NODE_API_VERSION = "1.2.1"
#
# 1.2.2 makes `tx_callsign` nullable, so a node whose owner cannot name the
# illuminator can register without one being invented for it. A patch on the
# test 1.1.3 applied to the same change on the coordinates: the document gains
# no field a client can read, and what a client generating from it sees is a
# type widening on a field it already had.
#
# The empty string stays refused. Null is the one way to say the illuminator is
# unnamed, which is what keeps a stored name distinguishable from its absence.
NODE_API_VERSION = "1.2.2"

# No tag here: each sub-router carries the contract's own grouping, since those
# are what a generated client is built around.
Expand Down
39 changes: 32 additions & 7 deletions backend/services/node_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,13 @@ def __init__(self, field: str, reason: str = "out of range") -> None:
# antenna characterised, so null is what the whole fleet sends for both.
_NULLABLE_BEAM = {"beam_width_deg", "beam_azimuth_deg"}

# Nullable since 1.2.2, on the coordinates' reasoning above: an owner who cannot
# name the illuminator has nothing to put here, and a substituted name is wrong
# data the server could not later tell apart from a real one. The empty string
# stays refused, so null is the single spelling of "unknown" and no stored row
# can hold the other.
_NULLABLE_CALLSIGN = {"tx_callsign"}

_SCHEMA_DESCRIPTION = """\
The receiver and illuminator geometry, the radio parameters and the association
tolerances. Every field is required.
Expand All @@ -89,6 +96,10 @@ def __init__(self, field: str, reason: str = "out of range") -> None:
it places nothing on the map until a position arrives. A latitude and its
longitude are given together or both null.

`tx_callsign` is nullable for the same reason, and the empty string is not: a
node that cannot name its illuminator sends null, which is the only way to say
so.

Necessary but not sufficient. A receiver and illuminator at the same point are
refused, as is a value that is not a finite number, and neither is expressible
here: both answer `400 invalid_config` naming the field."""
Expand Down Expand Up @@ -121,7 +132,7 @@ def config_json_schema() -> dict[str, Any]:
"""
properties = {field: _numeric_property(*bounds) for field, bounds in _NUMERIC_BOUNDS.items()}
properties |= deepcopy(_UNTABLED_PROPERTIES)
nullable = _NULLABLE | _NULLABLE_BEAM
nullable = _NULLABLE | _NULLABLE_BEAM | _NULLABLE_CALLSIGN
return {
"type": "object",
"title": "NodeConfig",
Expand All @@ -139,6 +150,13 @@ def config_json_schema() -> dict[str, Any]:
}


def _typed_branch(published: dict[str, Any], json_type: str) -> dict[str, Any]:
for alternative in published.get("anyOf", [published]):
if alternative.get("type") == json_type:
return alternative
return published


def numeric_branch(published: dict[str, Any]) -> dict[str, Any]:
"""The number half of a published property, whether or not it is nullable.

Expand All @@ -147,10 +165,12 @@ def numeric_branch(published: dict[str, Any]) -> dict[str, Any]:
the type rather than on position in the `anyOf`, so reordering the branches
cannot leave a caller reading the null one and finding no bounds at all.
"""
for alternative in published.get("anyOf", [published]):
if alternative.get("type") == "number":
return alternative
return published
return _typed_branch(published, "number")


def string_branch(published: dict[str, Any]) -> dict[str, Any]:
"""numeric_branch for the one field whose bounds are lengths, not magnitudes."""
return _typed_branch(published, "string")


def _as_finite_float(value: Any) -> tuple[float | None, str]:
Expand Down Expand Up @@ -211,10 +231,15 @@ def validate_config(payload: dict[str, Any]) -> dict[str, Any]:
raise ConfigInvalid(field)
out[field] = value

# Null is the illuminator being unnamed, and "" is not a shorter way to say
# it: see _NULLABLE_CALLSIGN.
callsign = payload["tx_callsign"]
if not isinstance(callsign, str) or not 1 <= len(callsign) <= 32:
if callsign is None:
out["tx_callsign"] = None
elif isinstance(callsign, str) and 1 <= len(callsign) <= 32:
out["tx_callsign"] = callsign
else:
raise ConfigInvalid("tx_callsign")
out["tx_callsign"] = callsign

# Required and nullable since 1.1.1: no node has its antenna characterised,
# because retina-gui does not collect the geometry from owners, so null is what
Expand Down
13 changes: 13 additions & 0 deletions backend/tests/test_node_config_endpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,19 @@ async def test_a_resend_with_both_antenna_fields_null_is_not_a_change(registered
assert (rows[1].beam_width_deg, rows[1].beam_azimuth_deg) == (None, None)


async def test_a_null_callsign_is_accepted_and_stored(registered_node, node_client, node_session):
"""An owner who cannot name the illuminator, which retina-gui otherwise fills in
with a placeholder the server could not tell apart from a real name."""
token, node_id = registered_node

response = node_client.put("/v1/nodes/config", headers=_auth(token), json=dict(CONFIG, tx_callsign=None))

assert response.status_code == 200
assert response.json() == {"config_version": 2}
rows = await _versions(node_session, node_id)
assert rows[-1].tx_callsign is None


async def test_a_changed_field_mints_the_next_version_and_supersedes_the_last(
registered_node, node_client, node_session
):
Expand Down
15 changes: 15 additions & 0 deletions backend/tests/test_node_config_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -269,3 +269,18 @@ async def test_a_null_position_round_trips(node_session):
await node_session.refresh(row)
assert row.rx_lat is None
assert row.tx_lat == 34.90


async def test_a_null_callsign_round_trips(node_session):
"""The column carries what contract 1.2.2 accepts: a node whose owner cannot
name the illuminator. Minting and comparison need nothing of their own for it,
since both are already null-aware for the antenna fields above."""
node_session.add(Node(node_id="test-null-sign", node_ref=mint_node_ref(), board_model="raspberrypi5-4gb"))
await node_session.flush()

version = await upsert_config(node_session, "test-null-sign", _config(tx_callsign=None))
named = await upsert_config(node_session, "test-null-sign", _config(tx_callsign="Wrotham"))

assert (version, named) == (1, 2)
rows = await _rows(node_session, "test-null-sign")
assert [row.tx_callsign for row in rows] == [None, "Wrotham"]
14 changes: 12 additions & 2 deletions backend/tests/test_node_config_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
config_json_schema,
numeric_branch,
position_status,
string_branch,
validate_config,
)

Expand Down Expand Up @@ -117,6 +118,8 @@ def test_zero_beam_azimuth_is_not_null():
("delay_tolerance_us", -1),
("doppler_tolerance_hz", 0),
("doppler_tolerance_hz", -1),
# Still refused now that null is accepted: null is the one spelling of
# "unknown", and services/node_config.py says why.
("tx_callsign", ""),
("tx_callsign", "x" * 33),
],
Expand Down Expand Up @@ -197,13 +200,20 @@ def test_a_non_numeric_type_is_rejected(value):
assert excinfo.value.field == "rx_lat"


@pytest.mark.parametrize("value", [123, None, ["CRYSTAL_PALACE"]])
@pytest.mark.parametrize("value", [123, ["CRYSTAL_PALACE"]])
def test_a_non_string_callsign_is_rejected(value):
with pytest.raises(ConfigInvalid) as excinfo:
validate_config(dict(VALID, tx_callsign=value))
assert excinfo.value.field == "tx_callsign"


def test_a_null_callsign_is_preserved_not_defaulted():
"""Nullable since contract 1.2.2, for the reason the coordinates are: an owner
who cannot name the illuminator says so, rather than a placeholder the server
could not later tell apart from a real name."""
assert validate_config(dict(VALID, tx_callsign=None))["tx_callsign"] is None


@pytest.mark.parametrize(
"field,value",
[
Expand Down Expand Up @@ -700,7 +710,7 @@ def test_every_published_bound_is_where_the_validator_refuses(field):


def test_the_published_callsign_length_is_where_the_validator_refuses():
schema = config_json_schema()["properties"]["tx_callsign"]
schema = string_branch(config_json_schema()["properties"]["tx_callsign"])

for length in (schema["minLength"], schema["maxLength"]):
validate_config(dict(VALID, tx_callsign="x" * length))
Expand Down
14 changes: 11 additions & 3 deletions backend/tests/test_node_openapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,14 @@

from routes.nodes import NODE_API_SERVERS, NODE_API_VERSION
from scripts.generate_openapi import CONTRACT_PATH, contract, render
from services.node_config import _NULLABLE, _NULLABLE_BEAM, _NUMERIC_BOUNDS, _REQUIRED, numeric_branch
from services.node_config import (
_NULLABLE,
_NULLABLE_BEAM,
_NULLABLE_CALLSIGN,
_NUMERIC_BOUNDS,
_REQUIRED,
numeric_branch,
)

FRAME = {
"t": 1753900000.123,
Expand Down Expand Up @@ -254,13 +261,14 @@ def test_every_bound_the_validator_enforces_reaches_the_schema(document):

def test_the_nullable_fields_publish_a_null_branch(document):
"""The six coordinates, so a node whose owner cannot supply the geometry
still registers, and the two beam fields, which no node has characterised.
still registers, the two beam fields, which no node has characterised, and
the callsign, which an owner who cannot name the illuminator leaves null.
A client generated from a document that omitted these cannot express the
config the fleet actually sends."""
properties = _published_config(document)["properties"]
nullable = {field for field, published in properties.items() if {"type": "null"} in published.get("anyOf", [])}

assert nullable == _NULLABLE | _NULLABLE_BEAM
assert nullable == _NULLABLE | _NULLABLE_BEAM | _NULLABLE_CALLSIGN


def test_the_contact_operation_reaches_its_own_component(document):
Expand Down
14 changes: 10 additions & 4 deletions contracts/nodes-v1.openapi.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
openapi: 3.1.0
info:
title: RETINA node ingest
version: 1.2.1
version: 1.2.2
description: |
The RETINA server's HTTP API. The paths under `/v1/nodes` are the RETINA node
ingest contract, generated from the server and versioned as a unit; everything
Expand Down Expand Up @@ -803,6 +803,10 @@ components:
it places nothing on the map until a position arrives. A latitude and its
longitude are given together or both null.

`tx_callsign` is nullable for the same reason, and the empty string is not: a
node that cannot name its illuminator sends null, which is the only way to say
so.

Necessary but not sufficient. A receiver and illuminator at the same point are
refused, as is a value that is not a finite number, and neither is expressible
here: both answer `400 invalid_config` naming the field.
Expand Down Expand Up @@ -866,9 +870,11 @@ components:
type: number
exclusiveMinimum: 0.0
tx_callsign:
type: string
minLength: 1
maxLength: 32
anyOf:
- type: string
minLength: 1
maxLength: 32
- type: 'null'
beam_width_deg:
anyOf:
- type: number
Expand Down
Loading