diff --git a/src/named_systems2.jl b/src/named_systems2.jl index ebe0e210..2e71041e 100644 --- a/src/named_systems2.jl +++ b/src/named_systems2.jl @@ -17,6 +17,21 @@ function check_unique(vals, s, msg=""; throw = true) return true end +_check_external_names(::Any, _allnames, _argname, _label, _hint) = nothing +function _check_external_names(names::AbstractVector{Symbol}, allnames, argname, label, hint) + snames = string.(allnames) + for name in names + name in allnames && continue + matches = allnames[findall(startswith(string(name)), snames)] + if isempty(matches) + error("The name $name in `$argname` was not found among the $label. Available are $(allnames)$hint.") + elseif length(matches) > 1 + error("The name $name in `$argname` is ambiguous: it prefix-matches multiple $label $matches. Use the exact name to disambiguate$hint.") + end + end + return nothing +end + function check_all_unique(s1, s2; throw=true) valx = check_unique([getproperty(s1, :x); getproperty(s2, :x)], "x"; throw) check_unique([getproperty(s1, :u); getproperty(s2, :u)], "u"; throw=true) @@ -675,6 +690,9 @@ function connect(systems; u1::Vector{Symbol}, y1::Vector{Symbol}, external_input check_unique(full.y, "system outputs") + _check_external_names(w1, full.u, "external_inputs", "system inputs", "; insert a `splitter(name, n)` to expose a fresh external signal connected to multiple ports") + _check_external_names(z1, full.y, "external_outputs", "system outputs", "") + if verbose leftover_inputs = setdiff(full.u, [u1; w1]) isempty(leftover_inputs) || @warn("The following inputs were unconnected $leftover_inputs, ignore this warning if you rely on prefix matching. Turn off this warning by passing `verbose = false`.") diff --git a/test/test_named_systems2.jl b/test/test_named_systems2.jl index b3e1cdb5..fc55b537 100644 --- a/test/test_named_systems2.jl +++ b/test/test_named_systems2.jl @@ -398,6 +398,42 @@ nss = named_ss(tf(1, [1,1])) @test nss isa NamedStateSpace +## Issue #114 — `connect` must reject ambiguous or unknown +## `external_inputs`/`external_outputs` names. Previously such names silently +## prefix-matched multiple internal ports and produced wrong externals. +sys_y1 = named_ss(ss(-1.0), u=:yinu, y=:youta) +sys_y2 = named_ss(ss( 1.0), u=:yiny, y=:youtb) + +# :y is supposed to be external but prefix-matches BOTH :yinu and :yiny +@test_throws "external_inputs" connect( + [sys_y1, sys_y2], + Pair{Symbol,Symbol}[]; + external_inputs = [:y], + external_outputs = [:youta, :youtb], +) + +# Symmetric: :yout prefix-matches both :youta and :youtb +@test_throws "external_outputs" connect( + [sys_y1, sys_y2], + Pair{Symbol,Symbol}[]; + external_inputs = [:yinu, :yiny], + external_outputs = [:yout], +) + +# Unknown name (no exact, no prefix) errors too +@test_throws "external_inputs" connect( + [sys_y1, sys_y2], + Pair{Symbol,Symbol}[]; + external_inputs = [:totally_bogus], + external_outputs = [:youta, :youtb], +) + +# Single-prefix match (e.g. `:x` -> `Symbol("x(t)")`) must still work — used by LQG tests +sys_a = named_ss(ssrand(1, 1, 1), u=Symbol("x(t)"), y=Symbol("y(t)"), x=:xa) +sys_b = named_ss(ssrand(1, 1, 1), u=:u_b, y=:y_b, x=:xb) +@test connect([sys_a, sys_b], [Symbol("y(t)") => :u_b]; + external_inputs = [:x], external_outputs = [:y_b]) isa NamedStateSpace + ## Test where one external input goes to several inputs with the same name, with and without using a splitter s1 = ssrand(1,2,2)