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

@@ -441,13 +441,7 @@ class CoreGrpcClient:
         return node, ifaces, links
 
     def edit_node(
-        self,
+        self, session_id: int, node_id: int, icon: str = None, source: str = None
-        session_id: int,
-        node_id: int,
-        position: wrappers.Position = None,
-        icon: str = None,
-        geo: wrappers.Geo = 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,
+            session_id=session_id, node_id=node_id, icon=icon, source=source
-            node_id=node_id,
-            position=position_proto,
-            icon=icon,
-            source=source,
-            geo=geo_proto,
         )
         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.

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"):
+            geo = request.geo if request.HasField("geo") else None
-                raise CoreError("move nodes must provide a move type")
+            position = request.position if request.HasField("position") else None
-            session = self.get_session(request.session_id, context)
+            self.move_node(
-            node = self.get_node(session, request.node_id, context, NodeBase)
+                context,
-            options = NodeOptions()
+                request.session_id,
-            has_geo = request.HasField("geo")
+                request.node_id,
-            if has_geo:
+                geo,
-                logger.info("has geo")
+                position,
-                lat = request.geo.lat
+                request.source,
-                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)
         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)
+        node.icon = request.icon or None
-        if request.HasField("position"):
+        source = request.source or None
-            x = request.position.x
+        session.broadcast_node(node, source=source)
-            y = request.position.y
+        return core_pb2.EditNodeResponse(result=True)
-            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)
 
     def DeleteNode(
         self, request: core_pb2.DeleteNodeRequest, context: ServicerContext

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:
+            has_geo = all(
-                if message.flags & MessageFlags.STRING.value:
+                i is not None for i in [options.lon, options.lat, options.alt]
-                    self.node_status_request[node.id] = True
+            )
-
+            if has_geo:
-                if self.session.state == EventTypes.RUNTIME_STATE:
+                self.session.broadcast_node(node)
-                    self.send_node_emulation_id(node.id)
+            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
 

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:
+    def set_node_pos(self, node: NodeBase, x: float, y: float) -> None:
-        """
+        node.setposition(x, y, None)
-        Edit node information.
+        self.sdt.edit_node(
+            node, node.position.lon, node.position.lat, node.position.alt
+        )
 
-        :param node_id: id of node to update
+    def set_node_geo(self, node: NodeBase, lon: float, lat: float, alt: float) -> None:
-        :param options: data to update node with
+        x, y, _ = self.location.getxyz(lat, lon, alt)
-        :return: nothing
+        if math.isinf(x) or math.isinf(y):
-        :raises core.CoreError: when node to update does not exist
+            raise CoreError(
-        """
+                f"invalid geo for current reference/scale: {lon},{lat},{alt}"
-        node = self.get_node(node_id, NodeBase)
+            )
-        node.icon = options.icon
+        node.setposition(x, y, None)
-        self.set_node_position(node, options)
+        node.position.set_geo(lon, lat, alt)
-        self.sdt.edit_node(node, options.lon, options.lat, options.alt)
+        self.sdt.edit_node(node, lon, lat, 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 start_mobility(self, node_ids: List[int] = None) -> None:
         """

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:

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 = 3;
-    string icon = 4;
+    string source = 4;
-    string source = 5;
-    Geo geo = 6;
 }
 
 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;

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)
+    result = core.move_node(session_id, args.id, pos, geo)
-    print(f"edit node: {result}")
+    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")
+    edit_parser.add_argument("-i", "--id", type=int, help="id to use", required=True)
-    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("-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)

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, icon)
-            result = client.edit_node(session.id, node.id, position)
 
         # then
         assert result is True
-        assert node.position.x == x
+        assert node.icon == icon
-        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)
 
     @pytest.mark.parametrize("node_id, expected", [(1, True), (2, False)])
     def test_delete_node(

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
+        x, y = 100.0, 50.0
-        update_options = NodeOptions()
-        update_options.set_position(x=position_value, y=position_value)
 
         # 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.x == x
-        assert node.position.y == position_value
+        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