From b96dc621cde4426ed61c2c1c4f4048107acc50d5 Mon Sep 17 00:00:00 2001 From: Blake Harnden <32446120+bharnden@users.noreply.github.com> Date: Fri, 27 Aug 2021 16:58:44 -0700 Subject: [PATCH] grpc: refactoring for editing/moving nodes, they are now two separate processes, with specific logic to carry each out --- daemon/core/api/grpc/client.py | 54 +++++++++------ daemon/core/api/grpc/server.py | 96 +++++++++++++++------------ daemon/core/api/tlv/corehandlers.py | 26 ++++++-- daemon/core/emulator/session.py | 62 ++++++----------- daemon/core/gui/coreclient.py | 2 +- daemon/proto/core/api/grpc/core.proto | 23 +++++-- daemon/scripts/core-cli | 24 +++++-- daemon/tests/test_grpc.py | 65 ++++++++++++------ daemon/tests/test_nodes.py | 25 +++++-- 9 files changed, 227 insertions(+), 150 deletions(-) diff --git a/daemon/core/api/grpc/client.py b/daemon/core/api/grpc/client.py index 6057d655..e3aab86a 100644 --- a/daemon/core/api/grpc/client.py +++ b/daemon/core/api/grpc/client.py @@ -441,13 +441,7 @@ class CoreGrpcClient: return node, ifaces, links def edit_node( - self, - session_id: int, - node_id: int, - position: wrappers.Position = None, - icon: str = None, - geo: wrappers.Geo = None, - source: str = None, + self, session_id: int, node_id: int, icon: str = None, source: str = None ) -> bool: """ Edit a node's icon and/or location, can only use position(x,y) or @@ -455,28 +449,50 @@ class CoreGrpcClient: :param session_id: session id :param node_id: node id - :param position: x,y location for node :param icon: path to icon for gui to use for node - :param geo: lon,lat,alt location for node :param source: application source :return: True for success, False otherwise :raises grpc.RpcError: when session or node doesn't exist """ - if position and geo: - raise CoreError("cannot edit position and geo at same time") - position_proto = position.to_proto() if position else None - geo_proto = geo.to_proto() if geo else None request = core_pb2.EditNodeRequest( - session_id=session_id, - node_id=node_id, - position=position_proto, - icon=icon, - source=source, - geo=geo_proto, + session_id=session_id, node_id=node_id, icon=icon, source=source ) response = self.stub.EditNode(request) return response.result + def move_node( + self, + session_id: int, + node_id: int, + position: wrappers.Position = None, + geo: wrappers.Geo = None, + source: str = None, + ) -> bool: + """ + Move node using provided position or geo location. + + :param session_id: session id + :param node_id: node id + :param position: x,y position to move to + :param geo: geospatial position to move to + :param source: source generating motion + :return: nothing + :raises grpc.RpcError: when session or nodes do not exist + """ + if not position and not geo: + raise CoreError("must provide position or geo to move node") + position = position.to_proto() if position else None + geo = geo.to_proto() if geo else None + request = core_pb2.MoveNodeRequest( + session_id=session_id, + node_id=node_id, + position=position, + geo=geo, + source=source, + ) + response = self.stub.MoveNode(request) + return response.result + def move_nodes(self, streamer: MoveNodesStreamer) -> None: """ Stream node movements using the provided iterator. diff --git a/daemon/core/api/grpc/server.py b/daemon/core/api/grpc/server.py index ed77ceeb..d8bb8078 100644 --- a/daemon/core/api/grpc/server.py +++ b/daemon/core/api/grpc/server.py @@ -75,7 +75,7 @@ from core.api.grpc.wlan_pb2 import ( ) from core.emane.modelmanager import EmaneModelManager from core.emulator.coreemu import CoreEmu -from core.emulator.data import InterfaceData, LinkData, LinkOptions, NodeOptions +from core.emulator.data import InterfaceData, LinkData, LinkOptions from core.emulator.enumerations import ( EventTypes, ExceptionLevels, @@ -165,6 +165,26 @@ class CoreGrpcServer(core_pb2_grpc.CoreApiServicer): except CoreError as e: context.abort(grpc.StatusCode.NOT_FOUND, str(e)) + def move_node( + self, + context: ServicerContext, + session_id: int, + node_id: int, + geo: core_pb2.Geo = None, + position: core_pb2.Position = None, + source: str = None, + ): + if not geo and not position: + raise CoreError("move node must provide a geo or position to move") + session = self.get_session(session_id, context) + node = self.get_node(session, node_id, context, NodeBase) + if geo: + session.set_node_geo(node, geo.lon, geo.lat, geo.alt) + else: + session.set_node_pos(node, position.x, position.y) + source = source if source else None + session.broadcast_node(node, source=source) + def validate_service( self, name: str, context: ServicerContext ) -> Type[ConfigService]: @@ -544,6 +564,23 @@ class CoreGrpcServer(core_pb2_grpc.CoreApiServicer): links = get_links(node) return core_pb2.GetNodeResponse(node=node_proto, ifaces=ifaces, links=links) + def MoveNode( + self, request: core_pb2.MoveNodeRequest, context: ServicerContext + ) -> core_pb2.MoveNodeResponse: + """ + Move node, either by x,y position or geospatial. + + :param request: move node request + :param context: context object + :return: move nodes response + """ + geo = request.geo if request.HasField("geo") else None + position = request.position if request.HasField("position") else None + self.move_node( + context, request.session_id, request.node_id, geo, position, request.source + ) + return core_pb2.MoveNodeResponse(result=True) + def MoveNodes( self, request_iterator: Iterable[core_pb2.MoveNodesRequest], @@ -557,27 +594,16 @@ class CoreGrpcServer(core_pb2_grpc.CoreApiServicer): :return: move nodes response """ for request in request_iterator: - if not request.WhichOneof("move_type"): - raise CoreError("move nodes must provide a move type") - session = self.get_session(request.session_id, context) - node = self.get_node(session, request.node_id, context, NodeBase) - options = NodeOptions() - has_geo = request.HasField("geo") - if has_geo: - logger.info("has geo") - lat = request.geo.lat - lon = request.geo.lon - alt = request.geo.alt - options.set_location(lat, lon, alt) - else: - x = request.position.x - y = request.position.y - logger.info("has pos: %s,%s", x, y) - options.set_position(x, y) - session.edit_node(node.id, options) - source = request.source if request.source else None - if not has_geo: - session.broadcast_node(node, source=source) + geo = request.geo if request.HasField("geo") else None + position = request.position if request.HasField("position") else None + self.move_node( + context, + request.session_id, + request.node_id, + geo, + position, + request.source, + ) return core_pb2.MoveNodesResponse() def EditNode( @@ -593,28 +619,10 @@ class CoreGrpcServer(core_pb2_grpc.CoreApiServicer): logger.debug("edit node: %s", request) session = self.get_session(request.session_id, context) node = self.get_node(session, request.node_id, context, NodeBase) - options = NodeOptions(icon=request.icon) - if request.HasField("position"): - x = request.position.x - y = request.position.y - options.set_position(x, y) - has_geo = request.HasField("geo") - if has_geo: - lat = request.geo.lat - lon = request.geo.lon - alt = request.geo.alt - options.set_location(lat, lon, alt) - result = True - try: - session.edit_node(node.id, options) - source = None - if request.source: - source = request.source - if not has_geo: - session.broadcast_node(node, source=source) - except CoreError: - result = False - return core_pb2.EditNodeResponse(result=result) + node.icon = request.icon or None + source = request.source or None + session.broadcast_node(node, source=source) + return core_pb2.EditNodeResponse(result=True) def DeleteNode( self, request: core_pb2.DeleteNodeRequest, context: ServicerContext diff --git a/daemon/core/api/tlv/corehandlers.py b/daemon/core/api/tlv/corehandlers.py index a1c1d34a..73a42af6 100644 --- a/daemon/core/api/tlv/corehandlers.py +++ b/daemon/core/api/tlv/corehandlers.py @@ -716,12 +716,15 @@ class CoreHandler(socketserver.BaseRequestHandler): if message.flags & MessageFlags.ADD.value: node = self.session.add_node(_class, node_id, options) - if node: - if message.flags & MessageFlags.STRING.value: - self.node_status_request[node.id] = True - - if self.session.state == EventTypes.RUNTIME_STATE: - self.send_node_emulation_id(node.id) + has_geo = all( + i is not None for i in [options.lon, options.lat, options.alt] + ) + if has_geo: + self.session.broadcast_node(node) + if message.flags & MessageFlags.STRING.value: + self.node_status_request[node.id] = True + if self.session.state == EventTypes.RUNTIME_STATE: + self.send_node_emulation_id(node.id) elif message.flags & MessageFlags.DELETE.value: with self._shutdown_lock: result = self.session.delete_node(node_id) @@ -739,7 +742,16 @@ class CoreHandler(socketserver.BaseRequestHandler): replies.append(coreapi.CoreNodeMessage.pack(flags, tlvdata)) # node update else: - self.session.edit_node(node_id, options) + node = self.session.get_node(node_id, NodeBase) + node.icon = options.icon + has_geo = all( + i is not None for i in [options.lon, options.lat, options.alt] + ) + if has_geo: + self.session.set_node_geo(node, options.lon, options.lat, options.alt) + self.session.broadcast_node(node) + else: + self.session.set_node_pos(node, options.x, options.y) return replies diff --git a/daemon/core/emulator/session.py b/daemon/core/emulator/session.py index 514bc168..72161717 100644 --- a/daemon/core/emulator/session.py +++ b/daemon/core/emulator/session.py @@ -545,7 +545,11 @@ class Session: node.canvas = options.canvas # set node position and broadcast it - self.set_node_position(node, options) + has_geo = all(i is not None for i in [options.lon, options.lat, options.alt]) + if has_geo: + self.set_node_geo(node, options.lon, options.lat, options.alt) + else: + self.set_node_pos(node, options.x, options.y) # add services to needed nodes if isinstance(node, (CoreNode, PhysicalNode)): @@ -583,49 +587,21 @@ class Session: self.sdt.add_node(node) return node - def edit_node(self, node_id: int, options: NodeOptions) -> None: - """ - Edit node information. + def set_node_pos(self, node: NodeBase, x: float, y: float) -> None: + node.setposition(x, y, None) + self.sdt.edit_node( + node, node.position.lon, node.position.lat, node.position.alt + ) - :param node_id: id of node to update - :param options: data to update node with - :return: nothing - :raises core.CoreError: when node to update does not exist - """ - node = self.get_node(node_id, NodeBase) - node.icon = options.icon - self.set_node_position(node, options) - self.sdt.edit_node(node, options.lon, options.lat, options.alt) - - def set_node_position(self, node: NodeBase, options: NodeOptions) -> None: - """ - Set position for a node, use lat/lon/alt if needed. - - :param node: node to set position for - :param options: data for node - :return: nothing - """ - # extract location values - x = options.x - y = options.y - lat = options.lat - lon = options.lon - alt = options.alt - # check if we need to generate position from lat/lon/alt - has_empty_position = all(i is None for i in [x, y]) - has_lat_lon_alt = all(i is not None for i in [lat, lon, alt]) - using_lat_lon_alt = has_empty_position and has_lat_lon_alt - if using_lat_lon_alt: - x, y, _ = self.location.getxyz(lat, lon, alt) - if math.isinf(x) or math.isinf(y): - raise CoreError( - f"invalid geo for current reference/scale: {lon},{lat},{alt}" - ) - node.setposition(x, y, None) - node.position.set_geo(lon, lat, alt) - self.broadcast_node(node) - elif not has_empty_position: - node.setposition(x, y, None) + def set_node_geo(self, node: NodeBase, lon: float, lat: float, alt: float) -> None: + x, y, _ = self.location.getxyz(lat, lon, alt) + if math.isinf(x) or math.isinf(y): + raise CoreError( + f"invalid geo for current reference/scale: {lon},{lat},{alt}" + ) + node.setposition(x, y, None) + node.position.set_geo(lon, lat, alt) + self.sdt.edit_node(node, lon, lat, alt) def start_mobility(self, node_ids: List[int] = None) -> None: """ diff --git a/daemon/core/gui/coreclient.py b/daemon/core/gui/coreclient.py index 63c20544..41753e5d 100644 --- a/daemon/core/gui/coreclient.py +++ b/daemon/core/gui/coreclient.py @@ -391,7 +391,7 @@ class CoreClient: def edit_node(self, core_node: Node) -> None: try: - self.client.edit_node( + self.client.move_node( self.session.id, core_node.id, core_node.position, source=GUI_SOURCE ) except grpc.RpcError as e: diff --git a/daemon/proto/core/api/grpc/core.proto b/daemon/proto/core/api/grpc/core.proto index 3fb55f73..df66254c 100644 --- a/daemon/proto/core/api/grpc/core.proto +++ b/daemon/proto/core/api/grpc/core.proto @@ -49,6 +49,8 @@ service CoreApi { } rpc GetNodeTerminal (GetNodeTerminalRequest) returns (GetNodeTerminalResponse) { } + rpc MoveNode (MoveNodeRequest) returns (MoveNodeResponse) { + } rpc MoveNodes (stream MoveNodesRequest) returns (MoveNodesResponse) { } @@ -329,10 +331,8 @@ message GetNodeResponse { message EditNodeRequest { int32 session_id = 1; int32 node_id = 2; - Position position = 3; - string icon = 4; - string source = 5; - Geo geo = 6; + string icon = 3; + string source = 4; } message EditNodeResponse { @@ -358,6 +358,21 @@ message GetNodeTerminalResponse { string terminal = 1; } + +message MoveNodeRequest { + int32 session_id = 1; + int32 node_id = 2; + string source = 3; + oneof move_type { + Position position = 4; + Geo geo = 5; + } +} + +message MoveNodeResponse { + bool result = 1; +} + message MoveNodesRequest { int32 session_id = 1; int32 node_id = 2; diff --git a/daemon/scripts/core-cli b/daemon/scripts/core-cli index f050cf90..fbbe6ede 100755 --- a/daemon/scripts/core-cli +++ b/daemon/scripts/core-cli @@ -268,6 +268,13 @@ def add_node(core: CoreGrpcClient, args: Namespace) -> None: @coreclient def edit_node(core: CoreGrpcClient, args: Namespace) -> None: + session_id = get_current_session(core, args.session) + result = core.edit_node(session_id, args.id, args.icon) + print(f"edit node: {result}") + + +@coreclient +def move_node(core: CoreGrpcClient, args: Namespace) -> None: session_id = get_current_session(core, args.session) pos = None if args.pos: @@ -277,8 +284,8 @@ def edit_node(core: CoreGrpcClient, args: Namespace) -> None: if args.geo: lon, lat, alt = args.geo geo = Geo(lon=lon, lat=lat, alt=alt) - result = core.edit_node(session_id, args.id, pos, args.icon, geo) - print(f"edit node: {result}") + result = core.move_node(session_id, args.id, pos, geo) + print(f"move node: {result}") @coreclient @@ -377,13 +384,18 @@ def setup_node_parser(parent: _SubParsersAction) -> None: edit_parser = subparsers.add_parser("edit", help="edit a node") edit_parser.formatter_class = ArgumentDefaultsHelpFormatter - edit_parser.add_argument("-i", "--id", type=int, help="id to use, optional") - group = edit_parser.add_mutually_exclusive_group(required=True) - group.add_argument("-p", "--pos", type=position_type, help="x,y position") - group.add_argument("-g", "--geo", type=geo_type, help="lon,lat,alt position") + edit_parser.add_argument("-i", "--id", type=int, help="id to use", required=True) edit_parser.add_argument("-ic", "--icon", help="icon to use, optional") edit_parser.set_defaults(func=edit_node) + move_parser = subparsers.add_parser("move", help="move a node") + move_parser.formatter_class = ArgumentDefaultsHelpFormatter + move_parser.add_argument("-i", "--id", type=int, help="id to use, optional", required=True) + group = move_parser.add_mutually_exclusive_group(required=True) + group.add_argument("-p", "--pos", type=position_type, help="x,y position") + group.add_argument("-g", "--geo", type=geo_type, help="lon,lat,alt position") + move_parser.set_defaults(func=move_node) + delete_parser = subparsers.add_parser("delete", help="delete a node") delete_parser.formatter_class = ArgumentDefaultsHelpFormatter delete_parser.add_argument("-i", "--id", type=int, help="node id", required=True) diff --git a/daemon/tests/test_grpc.py b/daemon/tests/test_grpc.py index ebfe29eb..575a502d 100644 --- a/daemon/tests/test_grpc.py +++ b/daemon/tests/test_grpc.py @@ -266,36 +266,63 @@ class TestGrpc: assert len(ifaces) == 0 assert len(links) == 0 + def test_move_node_pos(self, grpc_server: CoreGrpcServer): + # given + client = CoreGrpcClient() + session = grpc_server.coreemu.create_session() + node = session.add_node(CoreNode) + position = Position(x=100.0, y=50.0) + + # then + with client.context_connect(): + result = client.move_node(session.id, node.id, position=position) + + # then + assert result is True + assert node.position.x == position.x + assert node.position.y == position.y + + def test_move_node_geo(self, grpc_server: CoreGrpcServer): + # given + client = CoreGrpcClient() + session = grpc_server.coreemu.create_session() + node = session.add_node(CoreNode) + geo = Geo(lon=0.0, lat=0.0, alt=0.0) + + # then + with client.context_connect(): + result = client.move_node(session.id, node.id, geo=geo) + + # then + assert result is True + assert node.position.lon == geo.lon + assert node.position.lat == geo.lat + assert node.position.alt == geo.alt + + def test_move_node_exception(self, grpc_server: CoreGrpcServer): + # given + client = CoreGrpcClient() + session = grpc_server.coreemu.create_session() + node = session.add_node(CoreNode) + + # then and when + with pytest.raises(CoreError), client.context_connect(): + client.move_node(session.id, node.id) + def test_edit_node(self, grpc_server: CoreGrpcServer): # given client = CoreGrpcClient() session = grpc_server.coreemu.create_session() node = session.add_node(CoreNode) + icon = "test.png" # then - x, y = 10, 10 with client.context_connect(): - position = Position(x=x, y=y) - result = client.edit_node(session.id, node.id, position) + result = client.edit_node(session.id, node.id, icon) # then assert result is True - assert node.position.x == x - assert node.position.y == y - - def test_edit_node_exception(self, grpc_server: CoreGrpcServer): - # given - client = CoreGrpcClient() - session = grpc_server.coreemu.create_session() - node = session.add_node(CoreNode) - - # then - x, y = 10, 10 - with client.context_connect(): - position = Position(x=x, y=y) - geo = Geo(lat=0, lon=0, alt=0) - with pytest.raises(CoreError): - client.edit_node(session.id, node.id, position, geo=geo) + assert node.icon == icon @pytest.mark.parametrize("node_id, expected", [(1, True), (2, False)]) def test_delete_node( diff --git a/daemon/tests/test_nodes.py b/daemon/tests/test_nodes.py index 8ed21f27..3f0fbab1 100644 --- a/daemon/tests/test_nodes.py +++ b/daemon/tests/test_nodes.py @@ -24,19 +24,30 @@ class TestNodes: assert node.alive() assert node.up - def test_node_update(self, session: Session): + def test_node_set_pos(self, session: Session): # given node = session.add_node(CoreNode) - position_value = 100 - update_options = NodeOptions() - update_options.set_position(x=position_value, y=position_value) + x, y = 100.0, 50.0 # when - session.edit_node(node.id, update_options) + session.set_node_pos(node, x, y) # then - assert node.position.x == position_value - assert node.position.y == position_value + assert node.position.x == x + assert node.position.y == y + + def test_node_set_geo(self, session: Session): + # given + node = session.add_node(CoreNode) + lon, lat, alt = 0.0, 0.0, 0.0 + + # when + session.set_node_geo(node, lon, lat, alt) + + # then + assert node.position.lon == lon + assert node.position.lat == lat + assert node.position.alt == alt def test_node_delete(self, session: Session): # given