From a281ca5d4209b66b0044bb7c01dcbbcb255af3ae Mon Sep 17 00:00:00 2001 From: iopapamanoglou Date: Tue, 12 Nov 2024 09:28:58 +0100 Subject: [PATCH] clonable links --- src/faebryk/core/cpp/__init__.pyi | 6 ++ src/faebryk/core/cpp/include/graph/graph.hpp | 14 ++++ src/faebryk/core/cpp/include/graph/links.hpp | 20 ++++- src/faebryk/core/cpp/src/graph/graph.cpp | 21 ++++- .../core/cpp/src/graph/graphinterface.cpp | 6 ++ src/faebryk/core/cpp/src/graph/link.cpp | 6 ++ src/faebryk/core/cpp/src/graph/links.cpp | 83 ++++++++++++++++++- src/faebryk/core/cpp/src/main.cpp | 9 +- src/faebryk/core/moduleinterface.py | 40 ++++----- test/core/test_hierarchy_connect.py | 6 +- 10 files changed, 179 insertions(+), 32 deletions(-) diff --git a/src/faebryk/core/cpp/__init__.pyi b/src/faebryk/core/cpp/__init__.pyi index b3a54a95..2b283561 100644 --- a/src/faebryk/core/cpp/__init__.pyi +++ b/src/faebryk/core/cpp/__init__.pyi @@ -80,6 +80,8 @@ class GraphInterface: def connect(self, others: Sequence[GraphInterface]) -> None: ... @overload def connect(self, other: GraphInterface, link: Link) -> None: ... + @overload + def connect(self, others: Sequence[GraphInterface], link: Link) -> None: ... class GraphInterfaceHierarchical(GraphInterface): def __init__(self, is_parent: bool) -> None: ... @@ -129,6 +131,10 @@ class LinkDirectConditionalFilterResult(enum.Enum): class LinkDirectDerived(LinkDirectConditional): def __init__(self, arg: Path, /) -> None: ... +class LinkExists: + def existing_link(self) -> Link: ... + def new_link(self) -> Link: ... + class LinkFilteredException(Exception): pass diff --git a/src/faebryk/core/cpp/include/graph/graph.hpp b/src/faebryk/core/cpp/include/graph/graph.hpp index aefb6b7a..eacd5709 100644 --- a/src/faebryk/core/cpp/include/graph/graph.hpp +++ b/src/faebryk/core/cpp/include/graph/graph.hpp @@ -38,6 +38,17 @@ using GI_refs_weak = std::vector; using HierarchicalNodeRef = std::pair; using Link_weak_ref = Link *; +class LinkExists : public std::runtime_error { + private: + Link_ref existing_link; + Link_ref new_link; + + public: + LinkExists(Link_ref existing_link, Link_ref new_link, const std::string &msg); + Link_ref get_existing_link(); + Link_ref get_new_link(); +}; + class Node { public: struct NodeException : public std::runtime_error { @@ -138,6 +149,7 @@ class GraphInterface { void connect(GI_ref_weak other); void connect(GI_refs_weak others); void connect(GI_ref_weak other, Link_ref link); + void connect(GI_refs_weak others, Link_ref link); // TODO replace with set_node(Node_ref node, std::string name) void set_node(Node_ref node); Node_ref get_node(); @@ -160,11 +172,13 @@ class Link { protected: Link(); Link(GI_ref_weak from, GI_ref_weak to); + Link(const Link &other); public: std::pair get_connections(); virtual void set_connections(GI_ref_weak from, GI_ref_weak to); bool is_setup(); + virtual Link_ref clone() const = 0; }; struct Edge { diff --git a/src/faebryk/core/cpp/include/graph/links.hpp b/src/faebryk/core/cpp/include/graph/links.hpp index 6d18617b..756ce174 100644 --- a/src/faebryk/core/cpp/include/graph/links.hpp +++ b/src/faebryk/core/cpp/include/graph/links.hpp @@ -11,6 +11,8 @@ class LinkDirect : public Link { public: LinkDirect(); LinkDirect(GI_ref_weak from, GI_ref_weak to); + LinkDirect(const LinkDirect &other); + Link_ref clone() const override; }; class LinkParent : public Link { @@ -20,10 +22,11 @@ class LinkParent : public Link { public: LinkParent(); LinkParent(GraphInterfaceHierarchical *from, GraphInterfaceHierarchical *to); - + LinkParent(const LinkParent &other); void set_connections(GI_ref_weak from, GI_ref_weak to) override; GraphInterfaceHierarchical *get_parent(); GraphInterfaceHierarchical *get_child(); + Link_ref clone() const override; }; class LinkNamedParent : public LinkParent { @@ -33,8 +36,9 @@ class LinkNamedParent : public LinkParent { LinkNamedParent(std::string name); LinkNamedParent(std::string name, GraphInterfaceHierarchical *from, GraphInterfaceHierarchical *to); - + LinkNamedParent(const LinkNamedParent &other); std::string get_name(); + Link_ref clone() const override; }; class LinkPointer : public Link { @@ -44,15 +48,19 @@ class LinkPointer : public Link { public: LinkPointer(); LinkPointer(GI_ref_weak from, GraphInterfaceSelf *to); + LinkPointer(const LinkPointer &other); void set_connections(GI_ref_weak from, GI_ref_weak to) override; GraphInterface *get_pointer(); GraphInterfaceSelf *get_pointee(); + Link_ref clone() const override; }; class LinkSibling : public LinkPointer { public: LinkSibling(); LinkSibling(GI_ref_weak from, GraphInterfaceSelf *to); + LinkSibling(const LinkSibling &other); + Link_ref clone() const override; }; class LinkDirectConditional : public LinkDirect { @@ -81,16 +89,20 @@ class LinkDirectConditional : public LinkDirect { LinkDirectConditional(FilterF filter, bool needs_only_first_in_path); LinkDirectConditional(FilterF filter, bool needs_only_first_in_path, GI_ref_weak from, GI_ref_weak to); + LinkDirectConditional(const LinkDirectConditional &other); void set_connections(GI_ref_weak from, GI_ref_weak to) override; FilterResult run_filter(Path path); bool needs_to_check_only_first_in_path(); + Link_ref clone() const override; }; class LinkDirectShallow : public LinkDirectConditional { // TODO public: LinkDirectShallow(); + LinkDirectShallow(const LinkDirectShallow &other); + Link_ref clone() const override; }; class LinkDirectDerived : public LinkDirectConditional { @@ -98,10 +110,14 @@ class LinkDirectDerived : public LinkDirectConditional { static std::pair make_filter_from_path(Path path); + Path path; + public: LinkDirectDerived(Path path); LinkDirectDerived(Path path, std::pair filter); LinkDirectDerived(Path path, GI_ref_weak from, GI_ref_weak to); LinkDirectDerived(Path path, std::pair filter, GI_ref_weak from, GI_ref_weak to); + LinkDirectDerived(const LinkDirectDerived &other); + Link_ref clone() const override; }; diff --git a/src/faebryk/core/cpp/src/graph/graph.cpp b/src/faebryk/core/cpp/src/graph/graph.cpp index 14ab337d..b0c4400e 100644 --- a/src/faebryk/core/cpp/src/graph/graph.cpp +++ b/src/faebryk/core/cpp/src/graph/graph.cpp @@ -45,11 +45,10 @@ void Graph::add_edge(Link_ref link) { auto G = Graph::merge_graphs(from->G, to->G); - // remove existing link + // existing link if (G->e_cache_simple[from].contains(to)) { - // this->remove_edge(this->e_cache[from][to]); - // TODO: reconsider this - throw std::runtime_error("link already exists"); + // handle policy in the caller + throw LinkExists(G->e_cache[from][to], link, "link already exists"); } G->e_cache_simple[from].insert(to); @@ -208,3 +207,17 @@ Set Graph::get_gifs() { std::vector> Graph::all_edges() { return this->e; } + +LinkExists::LinkExists(Link_ref existing_link, Link_ref new_link, const std::string &msg) + : std::runtime_error(msg) + , existing_link(existing_link) + , new_link(new_link) { +} + +Link_ref LinkExists::get_existing_link() { + return this->existing_link; +} + +Link_ref LinkExists::get_new_link() { + return this->new_link; +} diff --git a/src/faebryk/core/cpp/src/graph/graphinterface.cpp b/src/faebryk/core/cpp/src/graph/graphinterface.cpp index 43d37c2a..00759c0e 100644 --- a/src/faebryk/core/cpp/src/graph/graphinterface.cpp +++ b/src/faebryk/core/cpp/src/graph/graphinterface.cpp @@ -76,6 +76,12 @@ void GraphInterface::connect(GI_ref_weak other, Link_ref link) { Graph::add_edge(link); } +void GraphInterface::connect(GI_refs_weak others, Link_ref link) { + for (auto other : others) { + this->connect(other, link->clone()); + } +} + void GraphInterface::register_graph(std::shared_ptr gi) { this->G->hold(gi); } diff --git a/src/faebryk/core/cpp/src/graph/link.cpp b/src/faebryk/core/cpp/src/graph/link.cpp index db84246a..639ab351 100644 --- a/src/faebryk/core/cpp/src/graph/link.cpp +++ b/src/faebryk/core/cpp/src/graph/link.cpp @@ -17,6 +17,12 @@ Link::Link(GI_ref_weak from, GI_ref_weak to) , setup(true) { } +Link::Link(const Link &other) + : from(nullptr) + , to(nullptr) + , setup(false) { +} + std::pair Link::get_connections() { if (!this->setup) { throw std::runtime_error("link not setup"); diff --git a/src/faebryk/core/cpp/src/graph/links.cpp b/src/faebryk/core/cpp/src/graph/links.cpp index fed06587..3df485db 100644 --- a/src/faebryk/core/cpp/src/graph/links.cpp +++ b/src/faebryk/core/cpp/src/graph/links.cpp @@ -13,6 +13,14 @@ LinkDirect::LinkDirect(GI_ref_weak from, GI_ref_weak to) : Link(from, to) { } +LinkDirect::LinkDirect(const LinkDirect &other) + : Link(other) { +} + +Link_ref LinkDirect::clone() const { + return std::make_shared(*this); +} + // LinkParent -------------------------------------------------------------------------- LinkParent::LinkParent() : Link() @@ -27,6 +35,12 @@ LinkParent::LinkParent(GraphInterfaceHierarchical *from, GraphInterfaceHierarchi this->set_connections(from, to); } +LinkParent::LinkParent(const LinkParent &other) + : Link(other) + , parent(nullptr) + , child(nullptr) { +} + void LinkParent::set_connections(GI_ref_weak from, GI_ref_weak to) { auto from_h = dynamic_cast(from); auto to_h = dynamic_cast(to); @@ -61,6 +75,10 @@ GraphInterfaceHierarchical *LinkParent::get_child() { return this->child; } +Link_ref LinkParent::clone() const { + return std::make_shared(*this); +} + // LinkNamedParent --------------------------------------------------------------------- LinkNamedParent::LinkNamedParent(std::string name) : LinkParent() @@ -73,10 +91,19 @@ LinkNamedParent::LinkNamedParent(std::string name, GraphInterfaceHierarchical *f , name(name) { } +LinkNamedParent::LinkNamedParent(const LinkNamedParent &other) + : LinkParent(other) + , name(other.name) { +} + std::string LinkNamedParent::get_name() { return this->name; } +Link_ref LinkNamedParent::clone() const { + return std::make_shared(*this); +} + // LinkPointer ------------------------------------------------------------------------- LinkPointer::LinkPointer() : Link() @@ -91,6 +118,12 @@ LinkPointer::LinkPointer(GI_ref_weak from, GraphInterfaceSelf *to) this->set_connections(from, to); } +LinkPointer::LinkPointer(const LinkPointer &other) + : Link(other) + , pointee(nullptr) + , pointer(nullptr) { +} + void LinkPointer::set_connections(GI_ref_weak from, GI_ref_weak to) { auto from_s = dynamic_cast(from); auto to_s = dynamic_cast(to); @@ -122,6 +155,10 @@ GraphInterface *LinkPointer::get_pointer() { return this->pointer; } +Link_ref LinkPointer::clone() const { + return std::make_shared(*this); +} + // LinkSibling ------------------------------------------------------------------------ LinkSibling::LinkSibling() : LinkPointer() { @@ -131,6 +168,14 @@ LinkSibling::LinkSibling(GI_ref_weak from, GraphInterfaceSelf *to) : LinkPointer(from, to) { } +LinkSibling::LinkSibling(const LinkSibling &other) + : LinkPointer(other) { +} + +Link_ref LinkSibling::clone() const { + return std::make_shared(*this); +} + // LinkDirectConditional ---------------------------------------------------------------- LinkDirectConditional::LinkDirectConditional(FilterF filter, bool needs_only_first_in_path) @@ -148,6 +193,12 @@ LinkDirectConditional::LinkDirectConditional(FilterF filter, this->set_connections(from, to); } +LinkDirectConditional::LinkDirectConditional(const LinkDirectConditional &other) + : LinkDirect(other) + , filter(other.filter) + , needs_only_first_in_path(other.needs_only_first_in_path) { +} + void LinkDirectConditional::set_connections(GI_ref_weak from, GI_ref_weak to) { if (this->filter(Path({from, to})) != FilterResult::FILTER_PASS) { throw LinkFilteredException("LinkDirectConditional filtered"); @@ -163,6 +214,10 @@ bool LinkDirectConditional::needs_to_check_only_first_in_path() { return this->needs_only_first_in_path; } +Link_ref LinkDirectConditional::clone() const { + return std::make_shared(*this); +} + // LinkDirectDerived ------------------------------------------------------------------- LinkDirectDerived::LinkDirectDerived(Path path) : LinkDirectDerived(path, make_filter_from_path(path)) { @@ -173,12 +228,23 @@ LinkDirectDerived::LinkDirectDerived(Path path, GI_ref_weak from, GI_ref_weak to } LinkDirectDerived::LinkDirectDerived(Path path, std::pair filter) - : LinkDirectConditional(filter.first, filter.second) { + : LinkDirectConditional(filter.first, filter.second) + , path(path) { } LinkDirectDerived::LinkDirectDerived(Path path, std::pair filter, GI_ref_weak from, GI_ref_weak to) - : LinkDirectConditional(filter.first, filter.second, from, to) { + : LinkDirectConditional(filter.first, filter.second, from, to) + , path(path) { +} + +LinkDirectDerived::LinkDirectDerived(const LinkDirectDerived &other) + : LinkDirectConditional(other) + , path(other.path) { +} + +Link_ref LinkDirectDerived::clone() const { + return std::make_shared(*this); } std::pair @@ -217,3 +283,16 @@ LinkDirectDerived::make_filter_from_path(Path path) { return {filterf, needs_only_first_in_path}; } + +// LinkDirectShallow ------------------------------------------------------------------- +// LinkDirectShallow::LinkDirectShallow() +// : LinkDirectConditional() { +//} +// +// LinkDirectShallow::LinkDirectShallow(const LinkDirectShallow &other) +// : LinkDirectConditional(other) { +//} +// +// Link_ref LinkDirectShallow::clone() const { +// return std::make_shared(*this); +//} diff --git a/src/faebryk/core/cpp/src/main.cpp b/src/faebryk/core/cpp/src/main.cpp index c81fca45..f0fff112 100644 --- a/src/faebryk/core/cpp/src/main.cpp +++ b/src/faebryk/core/cpp/src/main.cpp @@ -88,7 +88,9 @@ PYMOD(m) { .def("connect", nb::overload_cast(&GI::connect), "other"_a) .def("connect", nb::overload_cast(&GI::connect), "others"_a) .def("connect", nb::overload_cast(&GI::connect), - "other"_a, "link"_a), + "other"_a, "link"_a) + .def("connect", nb::overload_cast(&GI::connect), + "others"_a, "link"_a), &GraphInterface::factory); nb::class_(m, "Graph") @@ -105,6 +107,11 @@ PYMOD(m) { nb::rv_policy::reference) .def("__repr__", &Graph::repr); + nb::exception(m, "LinkExists"); + nb::class_(m, "LinkExists") + .def("existing_link", &LinkExists::get_existing_link, nb::rv_policy::reference) + .def("new_link", &LinkExists::get_new_link, nb::rv_policy::reference); + // Graph interfaces FACTORY((nb::class_(m, "GraphInterfaceSelf")), &GraphInterfaceSelf::factory); diff --git a/src/faebryk/core/moduleinterface.py b/src/faebryk/core/moduleinterface.py index 316eed40..b0572f87 100644 --- a/src/faebryk/core/moduleinterface.py +++ b/src/faebryk/core/moduleinterface.py @@ -3,6 +3,7 @@ import logging from itertools import pairwise from typing import ( + Iterable, Sequence, cast, ) @@ -25,7 +26,7 @@ from faebryk.core.pathfinder import find_paths from faebryk.core.trait import Trait from faebryk.library.can_specialize import can_specialize -from faebryk.libs.util import ConfigFlag, cast_assert, groupby, once, times +from faebryk.libs.util import ConfigFlag, cast_assert, groupby, once logger = logging.getLogger(__name__) @@ -85,7 +86,7 @@ def __init__(self): def __preinit__(self) -> None: ... def connect( - self: Self, *other: Self, linkcls: type[Link] | Link | None = None + self: Self, *other: Self, link: type[Link] | Link | None = None ) -> Self: if not {type(o) for o in other}.issubset({type(self)}): raise NodeException( @@ -99,36 +100,29 @@ def connect( # - pro: more intuitive ret = other[-1] if other else self - if linkcls is None or linkcls is LinkDirect: - self.connected.connect([o.connected for o in other]) - return ret + if link is None: + link = LinkDirect + if isinstance(link, type): + link = link() - # TODO: give link a proper copy constructor instead - if isinstance(linkcls, Link): - assert len(other) <= 1 - links = [linkcls] - else: - links = times(len(other), linkcls) - - for o, link in zip(other, links): - self.connected.connect(o.connected, link=link) + self.connected.connect([o.connected for o in other], link=link) return ret - def connect_via(self, bridge: Node | Sequence[Node], *other: Self, linkcls=None): + def connect_via(self, bridge: Node | Sequence[Node], *other: Self, link=None): from faebryk.library.can_bridge import can_bridge bridges = [bridge] if isinstance(bridge, Node) else bridge intf = self for sub_bridge in bridges: t = sub_bridge.get_trait(can_bridge) - intf.connect(t.get_in(), linkcls=linkcls) + intf.connect(t.get_in(), link=link) intf = t.get_out() - intf.connect(*other, linkcls=linkcls) + intf.connect(*other, link=link) def connect_shallow(self, *other: Self) -> Self: - return self.connect(*other, linkcls=type(self).LinkDirectShallow()) + return self.connect(*other, link=type(self).LinkDirectShallow()) def get_connected(self) -> dict[Self, Path]: paths = find_paths(self, []) @@ -187,7 +181,13 @@ def _path_with_least_conditionals(paths: list["Path"]) -> "Path": return paths[0] paths_links = [ - (path, [e1.is_connected_to(e2) for e1, e2 in pairwise(path)]) + ( + path, + [ + e1.is_connected_to(e2) + for e1, e2 in pairwise(cast(Iterable[GraphInterface], path)) + ], + ) for path in paths ] paths_conditionals = [ @@ -214,4 +214,4 @@ def _connect_via_implied_paths(self, other: Self, paths: list["Path"]): # heuristic: choose path with fewest conditionals path = self._path_with_least_conditionals(paths) - self.connect(other, linkcls=LinkDirectDerived(path)) + self.connect(other, link=LinkDirectDerived(path)) diff --git a/test/core/test_hierarchy_connect.py b/test/core/test_hierarchy_connect.py index 18978af3..c7a8c40f 100644 --- a/test/core/test_hierarchy_connect.py +++ b/test/core/test_hierarchy_connect.py @@ -316,7 +316,7 @@ def __init__(self): mifs = times(3, ModuleInterface) mifs_special = times(3, Specialized) - mifs[0].connect(mifs[1], linkcls=_Link) + mifs[0].connect(mifs[1], link=_Link) mifs[1].connect(mifs[2]) mifs[0].specialize(mifs_special[0]) @@ -366,7 +366,7 @@ def test_specialize_module(): def test_isolated_connect_simple(): x1 = F.ElectricLogic() x2 = F.ElectricLogic() - x1.connect(x2, linkcls=F.ElectricLogic.LinkIsolatedReference) + x1.connect(x2, link=F.ElectricLogic.LinkIsolatedReference) assert x1.is_connected_to(x2) assert x1.signal.is_connected_to(x2.signal) @@ -396,7 +396,7 @@ def test_isolated_connect_erc(): a1 = F.I2C() b1 = F.I2C() - a1.connect(b1, linkcls=F.ElectricLogic.LinkIsolatedReference) + a1.connect(b1, link=F.ElectricLogic.LinkIsolatedReference) assert a1.is_connected_to(b1) assert a1.scl.signal.is_connected_to(b1.scl.signal) assert a1.sda.signal.is_connected_to(b1.sda.signal)