Merge pull request #1316 from luxonis/fix/create_subnodes

Fix method responsible for creating python subnodes (createSubnode)

Author: lnotspotl

bindings/python/src/pipeline/node/HostNodeBindings.cpp

@@ -172,7 +172,11 @@ void bind_hostnode(pybind11::module& m, void* pCallstack) {
 
         # Create a subnode as part of this user node
         def createSubnode(self, class_, *args, **kwargs):
-            return self.getParentPipeline().create(class_, *args, **kwargs)
+            pipeline = self.getParentPipeline()
+            child_node = pipeline.create(class_, *args, **kwargs)
+            pipeline.remove(child_node) # necessary so that the node is only registered once
+            self.add(child_node)
+            return child_node
 
         node.HostNode.createSubnode = createSubnode
         node.ThreadedHostNode.createSubnode = createSubnode

bindings/python/src/pipeline/node/NodeBindings.cpp

@@ -430,7 +430,8 @@ void NodeBindings::bind(pybind11::module& m, void* pCallstack) {
         .def("getAssetManager",
              static_cast<AssetManager& (Node::*)()>(&Node::getAssetManager),
              py::return_value_policy::reference_internal,
-             DOC(dai, Node, getAssetManager));
+             DOC(dai, Node, getAssetManager))
+        .def("add", &Node::add, py::arg("node"), DOC(dai, Node, add));
 
     // TODO(themarpe) - refactor, threaded node could be separate from Node
     pyThreadedNode.def("trace", [](dai::ThreadedNode& node, const std::string& msg) { node.logger->trace(msg); })

src/pipeline/Node.cpp

@@ -587,9 +587,9 @@ void Node::add(std::shared_ptr<Node> node) {
 void Node::remove(std::shared_ptr<Node> node) {
     // Remove the connection to the removed node and all it's children from all the nodes in the pipeline
     auto pipeline = parent.lock();
-    if(pipeline == nullptr) {
-        throw std::runtime_error("Pipeline is null");
-    }
+
+    DAI_CHECK_V(pipeline != nullptr, "Pipeline is null");
+    DAI_CHECK_V(!pipeline->isBuilt(), "Cannot remove node from pipeline once it is built.");
 
     for(auto& n : pipeline->nodes) {
         for(auto& childNode : node->nodeMap) {

src/pipeline/Pipeline.cpp

@@ -13,6 +13,7 @@
 #include "pipeline/node/DetectionNetwork.hpp"
 #include "utility/Compression.hpp"
 #include "utility/Environment.hpp"
+#include "utility/ErrorMacros.hpp"
 #include "utility/HolisticRecordReplay.hpp"
 #include "utility/Logging.hpp"
 #include "utility/Platform.hpp"
@@ -438,13 +439,9 @@ BoardConfig PipelineImpl::getBoardConfig() const {
 
 // Remove node capability
 void PipelineImpl::remove(std::shared_ptr<Node> toRemove) {
-    if(toRemove->parent.lock() == nullptr) {
-        throw std::invalid_argument("Cannot remove a node that is not a part of any pipeline");
-    }
-
-    if(toRemove->parent.lock() != parent.pimpl) {
-        throw std::invalid_argument("Cannot remove a node that is not a part of this pipeline");
-    }
+    DAI_CHECK_V(!isBuilt(), "Cannot remove node from pipeline once it is built.");
+    DAI_CHECK_V(toRemove->parent.lock() != nullptr, "Cannot remove a node that is not a part of any pipeline");
+    DAI_CHECK_V(toRemove->parent.lock() == parent.pimpl, "Cannot remove a node that is not a part of this pipeline");
 
     // First remove the node from the pipeline directly
     auto it = std::remove(nodes.begin(), nodes.end(), toRemove);

src/pipeline/ThreadedNode.cpp

@@ -5,6 +5,7 @@
 #include "utility/Environment.hpp"
 #include "utility/Logging.hpp"
 #include "utility/Platform.hpp"
+#include "utility/ErrorMacros.hpp"
 
 namespace dai {
 ThreadedNode::ThreadedNode() {
@@ -17,6 +18,11 @@ ThreadedNode::ThreadedNode() {
 }
 
 void ThreadedNode::start() {
+
+    // A node should not be started if it is already running
+    // We would be creating multiple threads for the same node
+    DAI_CHECK_V(!isRunning(), "Node with id {} is already running. Cannot start it again. Node name: {}", id, getName());
+
     onStart();
     // Start the thread
     running = true;

src/pipeline/node/Camera.cpp

@@ -46,11 +46,19 @@ class Camera::Impl {
     }
 
     void buildStage1(Camera& parent) {
+        size_t numUnconnectedOutputs = 0;
         for(const auto& outputRequest : outputRequests) {
-            DAI_CHECK(!parent.dynamicOutputs[std::to_string(outputRequest.id)].getQueueConnections().empty()
-                          || !parent.dynamicOutputs[std::to_string(outputRequest.id)].getConnections().empty(),
-                      "Always call output->createOutputQueue() or output->link(*) after calling dai::node::Camera::requestOutput()");
+            bool hasQueue = !parent.dynamicOutputs[std::to_string(outputRequest.id)].getQueueConnections().empty();
+            bool hasConnections = !parent.dynamicOutputs[std::to_string(outputRequest.id)].getConnections().empty();
+            if(!hasQueue && !hasConnections) {
+                ++numUnconnectedOutputs;
+            }
         }
+
+        DAI_CHECK_V(numUnconnectedOutputs == 0,
+                    "Always call output->createOutputQueue() or output->link(*) after calling dai::node::Camera::requestOutput(). There is(are) {} requested "
+                    "Camera output(s) with no connected inputs or created output queues.",
+                    numUnconnectedOutputs);
     }
 };