From 480d24384b6eb188942cbb55396f49e212e09507 Mon Sep 17 00:00:00 2001 From: Gregory Salaun Date: Sun, 16 Aug 2026 00:09:18 +0200 Subject: [PATCH] fix(relays): {value} is the relay's label, not a per-relay value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It was built the other way round on a misreading of two screenshots: the pattern held {value} and the per-relay boxes held numbers to drop into it. The ask was simpler and better — {value} is the name typed in Relay labels, so a switch addressed by antenna name is one pattern instead of eight URLs: http://10.10.10.100/relay?on={value} relay 1 named Ant1 → ?on=Ant1 Renaming the antenna re-addresses it, and the name on the button and the name on the wire cannot drift apart because they are the same string. It works in the per-relay URLs and in the patterns alike, so the per-relay boxes go back to holding URLs and nothing about them changes meaning any more. The label is percent-encoded with %20 rather than "+" for a space: "+" is a space only in a query string and a literal plus in a path, and this can land in either half of a URL. {value} on a relay with no label would send "?on=", an empty parameter that most boards answer with a cheerful 200 and no movement. The driver refuses it and names the label as what is missing; the editor warns while it is being typed, beside the empty box rather than after an antenna fails to switch. The labels also join the driver's cache key — they are part of the wire format now. --- app.go | 8 +- changelog.json | 4 +- .../src/components/StationControlPanel.tsx | 24 ++-- frontend/src/lib/i18n.tsx | 8 +- internal/relaydev/httpgen.go | 125 ++++++++++-------- internal/relaydev/httpgen_test.go | 57 ++++---- 6 files changed, 128 insertions(+), 98 deletions(-) diff --git a/app.go b/app.go index 2038e5d..c27e53e 100644 --- a/app.go +++ b/app.go @@ -14763,7 +14763,7 @@ func buildDeviceDriver(d StationDevice) relaydev.Device { // generic board fall through to the WebSwitch driver below: it answered // the WebSwitch's own address, never sent one configured URL, and // reported itself offline so every relay button stayed greyed out. - return relaydev.NewHTTPGeneric(d.OnURLs, d.OffURLs, d.OnPat, d.OffPat, d.User, d.Pass, deviceRelayCount(d)) + return relaydev.NewHTTPGeneric(d.OnURLs, d.OffURLs, d.OnPat, d.OffPat, d.User, d.Pass, deviceRelayCount(d), d.Labels) default: return relaydev.NewWebswitch(d.Host) } @@ -14779,7 +14779,11 @@ func deviceKey(d StationDevice) string { // handed back the cached driver still holding the wrong address, so the // fix appeared to do nothing until OpsLog was restarted. k += "|" + d.OnPat + "|" + d.OffPat + - "|" + strings.Join(d.OnURLs, "\x1f") + "|" + strings.Join(d.OffURLs, "\x1f") + "|" + strings.Join(d.OnURLs, "\x1f") + "|" + strings.Join(d.OffURLs, "\x1f") + + // The labels are part of the wire format here: {value} sends them. + // Renaming a relay re-addresses it, and the cached driver would keep + // commanding the old name. + "|" + strings.Join(d.Labels, "\x1f") } return k } diff --git a/changelog.json b/changelog.json index 427fde8..8edd980 100644 --- a/changelog.json +++ b/changelog.json @@ -6,13 +6,13 @@ "WinKeyer: the opening probe goes out as one write, matching a capture of a client that talks to the same K3NG keyer, and the handshake bytes are always logged so a keyer that stays silent can be diagnosed.", "DX cluster: when spots arrive and every one is filtered out, the panel says so, names the filters doing it and offers to clear them — it used to say “waiting for spots” beside a counter reading 76 live.", "DX cluster: the log times the connection and the first spot, so a slow first launch can be told apart from a quiet node.", - "Generic HTTP relay: its URLs were never actually sent — fixed. Adds {value} and {relay-1}, https, and no host or test needed." + "Generic HTTP relay: its URLs were never actually sent — fixed. {value} sends the relay's label, {relay-1} counts from zero, no host needed." ], "fr": [ "WinKeyer : la sonde d’ouverture part en un seul envoi, calquée sur la capture d’un client qui dialogue avec le même manipulateur K3NG, et les octets de la poignée de main sont toujours journalisés pour diagnostiquer un manipulateur muet.", "Cluster DX : quand des spots arrivent et que tout est filtré, le panneau le dit, nomme les filtres responsables et propose de les effacer — il affichait « en attente de spots » à côté d’un compteur à 76 en direct.", "Cluster DX : le journal chronomètre la connexion et le premier spot, pour distinguer un premier lancement lent d’un nœud silencieux.", - "Relais HTTP générique : ses URL n’étaient jamais envoyées — corrigé. Ajoute {value} et {relay-1}, https, sans hôte ni test." + "Relais HTTP générique : ses URL n’étaient jamais envoyées — corrigé. {value} envoie le libellé du relais, {relay-1} compte de zéro, hôte inutile." ] }, { diff --git a/frontend/src/components/StationControlPanel.tsx b/frontend/src/components/StationControlPanel.tsx index 9778658..ffddad1 100644 --- a/frontend/src/components/StationControlPanel.tsx +++ b/frontend/src/components/StationControlPanel.tsx @@ -696,11 +696,13 @@ function DeviceEditor({ device, onChange, onSave, onCancel, t }: { const isDenkovi = device.type === 'denkovi'; const isUsbRelay = device.type === 'usbrelay'; const isHTTPGen = device.type === 'httpgen'; - // {value} in a pattern changes what the boxes below hold — a value to drop - // into it instead of a whole URL. The grid says which as soon as it is typed, - // because the two are indistinguishable once entered and getting it wrong - // switches an antenna somewhere unexpected. - const usesValue = `${device.on_pattern ?? ''}${device.off_pattern ?? ''}`.includes('{value}'); + // {value} sends a relay's label, so a URL using it on an unnamed relay would + // go out with an empty parameter. Warn while it is being typed rather than at + // the moment an antenna fails to switch. + const valueNeedsLabels = isHTTPGen + && [...(device.on_urls ?? []), ...(device.off_urls ?? []), device.on_pattern ?? '', device.off_pattern ?? ''] + .some((s) => (s ?? '').includes('{value}')) + && device.labels.some((l) => !l.trim()); // COM ports for the generic USB-serial relay picker. const [serialPorts, setSerialPorts] = useState([]); useEffect(() => { @@ -874,12 +876,12 @@ function DeviceEditor({ device, onChange, onSave, onCancel, t }: {
{t('station.patternHint')}
- +
{device.labels.map((_, i) => (
{i + 1} - { const on_urls = [...(device.on_urls ?? [])]; @@ -887,7 +889,7 @@ function DeviceEditor({ device, onChange, onSave, onCancel, t }: { on_urls[i] = e.target.value; onChange({ ...device, on_urls }); }} /> - { const off_urls = [...(device.off_urls ?? [])]; @@ -898,13 +900,17 @@ function DeviceEditor({ device, onChange, onSave, onCancel, t }: {
))}
-
{usesValue ? t('station.perValueHint') : t('station.perRelayHint')}
+
{t('station.perRelayHint')}
)}
+ {/* {value} sends the label, so an unnamed relay would go out as "?on=". + Said here, beside the empty box, rather than when the antenna fails + to switch and the log is the only place that explains why. */} + {valueNeedsLabels &&

{t('station.valueNeedsLabels')}

}
{device.labels.map((lab, i) => ( = 0 && i < len(h.labels) { + return strings.TrimSpace(h.labels[i]) + } + return "" +} + +// urlFor builds the request for one relay in one direction: the per-relay URL +// if there is one, the pattern otherwise, with both substitutions applied. func (h *httpGen) urlFor(relay int, on bool) string { - pat, entry := h.patFor(on), h.entryFor(relay, on) - if strings.Contains(pat, "{value}") { - if entry == "" { - return "" - } - return expandRelay(strings.ReplaceAll(pat, "{value}", entry), relay) + u := h.entryFor(relay, on) + if u == "" { + u = h.patFor(on) } - if entry != "" { - return expandRelay(entry, relay) - } - if pat == "" { + if u == "" { return "" } - return expandRelay(pat, relay) + return expand(u, relay, h.labelFor(relay)) +} + +// escapeValue percent-encodes a relay label for use anywhere in a URL. +// +// url.QueryEscape alone is wrong: it writes a space as "+", which is a space +// only in a query string and a literal plus sign in a path. Encoding it as %20 +// instead is correct in both, and {value} may land in either. +func escapeValue(s string) string { + return strings.ReplaceAll(url.QueryEscape(s), "+", "%20") } // withScheme supplies http:// when none was typed, and leaves https:// alone. @@ -139,10 +153,12 @@ func withScheme(u string) string { // relayToken matches {relay} and its offset forms, {relay-1} / {relay+2}. var relayToken = regexp.MustCompile(`\{relay([+-]\d+)?\}`) -// expandRelay substitutes the relay number, honouring an offset. A board that -// numbers its channels from zero is written {relay-1}; without that the whole -// pattern has to be abandoned for four hand-typed URLs. -func expandRelay(s string, relay int) string { +// expand substitutes {value} with the relay's label and {relay} with its +// number, honouring an offset. A board that numbers its channels from zero is +// written {relay-1}; without that the whole pattern has to be abandoned for +// four hand-typed URLs. +func expand(s string, relay int, label string) string { + s = strings.ReplaceAll(s, "{value}", escapeValue(label)) return relayToken.ReplaceAllStringFunc(s, func(m string) string { n := relay if i := strings.IndexAny(m, "+-"); i >= 0 { @@ -158,22 +174,27 @@ func (h *httpGen) Set(ctx context.Context, relay int, on bool) error { if relay < 1 || relay > h.count { return fmt.Errorf("relay %d out of range 1..%d", relay, h.count) } - u := h.urlFor(relay, on) - if u == "" { - // Naming the direction matters: an operator who filled the ON URLs and - // left OFF empty gets a switch that latches, and "no URL configured" - // alone would not say which half is missing. Name what is missing too — - // with {value} in the pattern the empty box wants a number, not a URL, - // and being told to enter a URL there sends them the wrong way. - dir, what := "OFF", "URL" - if on { - dir = "ON" - } - if strings.Contains(h.patFor(on), "{value}") { - what = "value" - } - return fmt.Errorf("no %s %s configured for relay %d", dir, what, relay) + // Naming the direction matters: an operator who filled the ON URLs and left + // OFF empty gets a switch that latches, and "no URL configured" alone would + // not say which half is missing. + dir := "OFF" + if on { + dir = "ON" } + tmpl := h.entryFor(relay, on) + if tmpl == "" { + tmpl = h.patFor(on) + } + if tmpl == "" { + return fmt.Errorf("no %s URL configured for relay %d", dir, relay) + } + // {value} with no label would send "?on=" — an empty parameter to an antenna + // switch, which most boards answer with a cheerful 200 and no movement. Say + // what is missing instead of firing it. + if strings.Contains(tmpl, "{value}") && h.labelFor(relay) == "" { + return fmt.Errorf("the %s URL for relay %d uses {value}, but relay %d has no label to put there", dir, relay, relay) + } + u := h.urlFor(relay, on) u = withScheme(u) if _, err := get(ctx, u, h.user, h.pass); err != nil { return err diff --git a/internal/relaydev/httpgen_test.go b/internal/relaydev/httpgen_test.go index aec2f03..0fe8306 100644 --- a/internal/relaydev/httpgen_test.go +++ b/internal/relaydev/httpgen_test.go @@ -22,7 +22,7 @@ func TestHTTPGenericPattern(t *testing.T) { d := NewHTTPGeneric(nil, nil, srv.URL+"/relay?n={relay}&state=on", - srv.URL+"/relay?n={relay}&state=off", "", "", 4) + srv.URL+"/relay?n={relay}&state=off", "", "", 4, nil) if err := d.Set(context.Background(), 2, true); err != nil { t.Fatalf("Set on: %v", err) } @@ -53,7 +53,7 @@ func TestHTTPGenericPerRelayURLsWinOverThePattern(t *testing.T) { d := NewHTTPGeneric( []string{srv.URL + "/FF0101", "", srv.URL + "/weird/on"}, []string{srv.URL + "/FF0100", "", ""}, - srv.URL+"/pattern/on/{relay}", srv.URL+"/pattern/off/{relay}", "", "", 3) + srv.URL+"/pattern/on/{relay}", srv.URL+"/pattern/off/{relay}", "", "", 3, nil) _ = d.Set(context.Background(), 1, true) // its own URL _ = d.Set(context.Background(), 2, true) // empty → falls back to the pattern @@ -66,36 +66,36 @@ func TestHTTPGenericPerRelayURLsWinOverThePattern(t *testing.T) { } } -// The {value} form: one address, and the per-relay boxes hold the number that -// goes into it. A bit-mask board (qro.cz) is the case — /Set0/1, /Set0/2, -// /Set0/4, /Set0/8 — where four full URLs differ by one character. -func TestHTTPGenericValueSubstitution(t *testing.T) { +// {value} is the relay's LABEL: a switch addressed by antenna name rather than +// by channel number is one pattern instead of eight URLs. +func TestHTTPGenericValueIsTheRelayLabel(t *testing.T) { var mu sync.Mutex var got []string srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { mu.Lock() - got = append(got, r.URL.Path) + got = append(got, r.URL.String()) mu.Unlock() })) defer srv.Close() d := NewHTTPGeneric( - []string{"1", "2", "4", "8"}, - []string{"0", "0", "0", "0"}, - srv.URL+"/Set0/{value}", srv.URL+"/Set0/{value}", "", "", 4) - _ = d.Set(context.Background(), 3, true) - _ = d.Set(context.Background(), 1, false) + []string{srv.URL + "/relay?on={value}"}, // per-relay URL + nil, + "", srv.URL+"/relay?off={value}", // and the pattern, for the other direction + "", "", 3, []string{"Ant1", "Beam 20m", ""}) + _ = d.Set(context.Background(), 1, true) + _ = d.Set(context.Background(), 2, false) mu.Lock() defer mu.Unlock() - want := []string{"/Set0/4", "/Set0/0"} + // The space in "Beam 20m" must go out as %20 — a "+" would be a literal plus + // in a path, and this substitution can land in either half of a URL. + want := []string{"/relay?on=Ant1", "/relay?off=Beam%2020m"} if strings.Join(got, " ") != strings.Join(want, " ") { t.Errorf("requested %v, want %v", got, want) } } -// {value} and {relay-1} together: the other API of the same board, whose -// channels are numbered from zero. Without the offset the pattern has to be -// abandoned for four hand-typed URLs. +// {relay-1} for a board whose channels are numbered from zero. func TestHTTPGenericRelayOffset(t *testing.T) { var mu sync.Mutex var got []string @@ -106,10 +106,8 @@ func TestHTTPGenericRelayOffset(t *testing.T) { })) defer srv.Close() - d := NewHTTPGeneric( - []string{"1", "1", "1", "1"}, - []string{"0", "0", "0", "0"}, - srv.URL+"/set0/{relay-1}/{value}", srv.URL+"/set0/{relay-1}/{value}", "", "", 4) + d := NewHTTPGeneric(nil, nil, + srv.URL+"/set0/{relay-1}/1", srv.URL+"/set0/{relay-1}/0", "", "", 4, nil) _ = d.Set(context.Background(), 1, true) _ = d.Set(context.Background(), 4, false) mu.Lock() @@ -120,14 +118,15 @@ func TestHTTPGenericRelayOffset(t *testing.T) { } } -// With {value} in the pattern the per-relay boxes hold values, so an empty one -// must be reported as a missing VALUE. Telling the operator to enter a URL in a -// box that wants "4" sends them to rewrite a configuration that was nearly right. -func TestHTTPGenericNamesAMissingValue(t *testing.T) { - d := NewHTTPGeneric(nil, nil, "http://x/Set0/{value}", "http://x/Set0/{value}", "", "", 2) +// A URL that uses {value} on an unlabelled relay would go out as "?on=" — an +// empty parameter, which most boards answer with a cheerful 200 and no +// movement. It must be refused, and the message must say the label is what is +// missing. +func TestHTTPGenericRefusesValueWithoutALabel(t *testing.T) { + d := NewHTTPGeneric(nil, nil, "http://x/relay?on={value}", "", "", "", 2, []string{"", ""}) err := d.Set(context.Background(), 1, true) - if err == nil || !strings.Contains(err.Error(), "value") { - t.Errorf("err = %v, want it to name the missing value", err) + if err == nil || !strings.Contains(err.Error(), "label") { + t.Errorf("err = %v, want it to name the missing label", err) } } @@ -150,7 +149,7 @@ func TestHTTPGenericSuppliesTheScheme(t *testing.T) { // A switch with the ON URLs filled and OFF left empty latches. The error has to // name the direction, or the operator cannot tell which half is missing. func TestHTTPGenericNamesTheMissingDirection(t *testing.T) { - d := NewHTTPGeneric([]string{"http://x/on"}, nil, "", "", "", "", 1) + d := NewHTTPGeneric([]string{"http://x/on"}, nil, "", "", "", "", 1, nil) err := d.Set(context.Background(), 1, false) if err == nil || !strings.Contains(err.Error(), "OFF") { t.Errorf("err = %v, want it to name the OFF direction", err) @@ -161,7 +160,7 @@ func TestHTTPGenericNamesTheMissingDirection(t *testing.T) { func TestHTTPGenericRemembersWhatItCommanded(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})) defer srv.Close() - d := NewHTTPGeneric(nil, nil, srv.URL+"/on/{relay}", srv.URL+"/off/{relay}", "", "", 3) + d := NewHTTPGeneric(nil, nil, srv.URL+"/on/{relay}", srv.URL+"/off/{relay}", "", "", 3, nil) _ = d.Set(context.Background(), 2, true) st, err := d.Status(context.Background()) if err != nil {