Rework the design, improve consistency

Signed-off-by: Michael X. Grey <[email protected]>

rmf_task/include/rmf_task/Request.hpp

@@ -36,6 +36,49 @@ namespace rmf_task {
 class Request
 {
 public:
+
+  class Tag
+  {
+  public:
+
+    /// Constructor
+    ///
+    /// \param[in] id_
+    ///   The identify of the request
+    ///
+    /// \param[in] earliest_start_time_
+    ///   The earliest time that the request may begin
+    ///
+    /// \param[in] priority_
+    ///   The priority of the request
+    ///
+    /// \param[in] automatic_
+    ///   Whether this request was automatically generated
+    Tag(
+      std::string id_,
+      rmf_traffic::Time earliest_start_time_,
+      ConstPriorityPtr priority_,
+      bool automatic_ = false);
+
+    /// The unique id for this request
+    const std::string& id() const;
+
+    /// Get the earliest time that this request may begin
+    rmf_traffic::Time earliest_start_time() const;
+
+    /// Get the priority of this request
+    ConstPriorityPtr priority() const;
+
+    // Returns true if this request was automatically generated
+    bool automatic() const;
+
+    class Implementation;
+  private:
+    rmf_utils::impl_ptr<Implementation> _pimpl;
+  };
+
+  using ConstTagPtr = std::shared_ptr<const Tag>;
+
   /// An abstract interface for computing the estimate and invariant durations
   /// of this request
   class Model
@@ -95,28 +138,30 @@ class Request
   ///
   /// \param[in] automatic
   ///   True if this request is auto-generated
+  //
+  // TODO(MXG): Deprecate this constructor?
   Request(
     const std::string& id,
     rmf_traffic::Time earliest_start_time,
     ConstPriorityPtr priority,
     ConstDescriptionPtr description,
     bool automatic = false);
 
-  /// The unique id for this request
-  const std::string& id() const;
-
-  /// Get the earliest time that this request may begin
-  rmf_traffic::Time earliest_start_time() const;
+  /// Constructor
+  ///
+  /// \param[in] tag
+  ///   Tag of the request
+  ///
+  /// \param[in] description
+  ///   Description for this request
+  Request(ConstTagPtr tag, ConstDescriptionPtr description);
 
-  /// Get the priority of this request
-  ConstPriorityPtr priority() const;
+  /// Get the tag of this request
+  const ConstTagPtr& tag() const;
 
   /// Get the description of this request
   const ConstDescriptionPtr& description() const;
 
-  // Returns true if this request was automatically generated
-  bool automatic() const;
-
   class Implementation;
 private:
   rmf_utils::impl_ptr<Implementation> _pimpl;

rmf_task/include/rmf_task/detail/Resume.hpp

@@ -0,0 +1,42 @@
+/*
+ * Copyright (C) 2021 Open Source Robotics Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+*/
+
+#ifndef RMF_TASK__DETAIL__RESUME_HPP
+#define RMF_TASK__DETAIL__RESUME_HPP
+
+#include <rmf_utils/impl_ptr.hpp>
+
+namespace rmf_task {
+namespace detail {
+
+//==============================================================================
+class Resume
+{
+public:
+
+  /// Call this object to tell the Task to resume.
+  void operator()() const;
+
+  class Implementation;
+private:
+  rmf_utils::unique_impl_ptr<Implementation> _pimpl;
+};
+
+} // namespace detail
+} // namespace rmf_task
+
+#endif // RMF_TASK__DETAIL__RESUME_HPP

rmf_task/include/rmf_task/execute/Phase.hpp

@@ -30,16 +30,64 @@ namespace rmf_task {
 namespace execute {
 
 //==============================================================================
-class CompletedPhase
+class Phase
 {
 public:
 
+  class Tag;
+  using ConstTagPtr = std::shared_ptr<const Tag>;
+
+  class Completed;
+  using ConstCompletedPtr = std::shared_ptr<const Completed>;
+
+  class Active;
+  using ConstActivePtr = std::shared_ptr<const Active>;
+
+  class Snapshot;
+  using ConstSnapshotPtr = std::shared_ptr<const Snapshot>;
+
+  class Pending;
+  using ConstPendingPtr = std::shared_ptr<const Pending>;
+};
+
+//==============================================================================
+/// Basic static information about a phase. This information should go
+/// unchanged from the Pending state, through the Active state, and into the
+/// Completed state.
+class Phase::Tag
+{
+public:
+
+  /// Name of the phase
   const std::string& name() const;
+
+  /// Details about the phase
   const std::string& detail() const;
+
+  /// The original (ideal) estimate of how long the phase will last
+  rmf_traffic::Duration original_duration_estimate() const;
+
+  class Implementation;
+private:
+  rmf_utils::impl_ptr<Implementation> _pimpl;
+};
+
+//==============================================================================
+/// Information about a phase that has been completed.
+class Phase::Completed
+{
+public:
+
+  /// Tag of the phase
+  const ConstTagPtr& tag() const;
+
+  /// The final log of the phase
   const Log::View& log() const;
 
+  /// The actual time that the phase started
   rmf_traffic::Time start_time() const;
-  rmf_traffic::Time original_finish_estimate() const;
+
+  /// The actual time that the phase finished.
   rmf_traffic::Time finish_time() const;
 
   class Implementation;
@@ -48,15 +96,12 @@ class CompletedPhase
 };
 
 //==============================================================================
-class ActivePhase
+class Phase::Active
 {
 public:
 
-  /// The name of this phase.
-  virtual std::string name() const = 0;
-
-  /// Details about this phase
-  virtual std::string detail() const = 0;
+  /// Tag of the phase
+  virtual ConstTagPtr tag() const = 0;
 
   /// The condition that needs to be satisfied for this phase to be complete
   virtual ConstConditionPtr finish_condition() const = 0;
@@ -65,19 +110,38 @@ class ActivePhase
   virtual rmf_traffic::Time estimate_finish_time() const = 0;
 
   // Virtual destructor
-  virtual ~ActivePhase() = default;
+  virtual ~Active() = default;
 };
 
-using ConstActivePhasePtr = std::shared_ptr<const ActivePhase>;
+//==============================================================================
+class Phase::Snapshot : public Phase::Active
+{
+public:
+
+  /// Make a snapshot of an Active phase
+  static ConstSnapshotPtr make(const Active& active);
+
+  // Documentation inherited
+  ConstTagPtr tag() const final;
+
+  // Documentation inherited
+  ConstConditionPtr finish_condition() const final;
+
+  // Documentation inherited
+  rmf_traffic::Time estimate_finish_time() const final;
+
+  class Implementation;
+private:
+  rmf_utils::impl_ptr<Implementation> _pimpl;
+};
 
 //==============================================================================
-class PendingPhase
+class Phase::Pending
 {
 public:
 
-  const std::string& name() const;
-  const std::string& detail() const;
-  rmf_traffic::Duration duration_estimate() const;
+  /// Tag of the phase
+  const ConstTagPtr& tag() const;
 
   class Implementation;
 private:

rmf_task/include/rmf_task/execute/Task.hpp

@@ -19,6 +19,7 @@
 #define RMF_TASK__EXECUTE__TASK_HPP
 
 #include <rmf_task/execute/Phase.hpp>
+#include <rmf_task/detail/Resume.hpp>
 #include <rmf_task/Request.hpp>
 
 #include <memory>
@@ -33,44 +34,48 @@ class Task
 {
 public:
 
-  /// Descriptions of the phases that have been completed
-  virtual const std::vector<CompletedPhase>& completed_phases() const = 0;
+  /// Basic static information about the task.
+  class Tag
+  {
+  public:
 
-  /// Interface for the phase that is currently active
-  virtual ConstActivePhasePtr active_phase() const = 0;
+    /// The original request that this Task is carrying out
+    const Request::ConstTagPtr& request() const;
 
-  /// Descriptions of the phases that are expected in the future
-  virtual std::vector<PendingPhase> pending_phases() const = 0;
+    /// The category of this Task.
+    const std::string& category() const;
+
+    /// Details about this Task.
+    const std::string& detail() const;
+
+    /// The original finish estimate of this Task.
+    rmf_traffic::Time original_finish_estimate() const;
+
+    class Implementation;
+  private:
+    rmf_utils::impl_ptr<Implementation> _pimpl;
+  };
 
-  /// The ID of this Task
-  virtual std::string id() const = 0;
+  /// Descriptions of the phases that have been completed
+  virtual const std::vector<Phase::ConstCompletedPtr>&
+  completed_phases() const = 0;
 
-  /// The category of this Task
-  virtual std::string category() const = 0;
+  /// Interface for the phase that is currently active
+  virtual Phase::ConstActivePtr active_phase() const = 0;
 
-  /// Human-readable details about this task
-  virtual std::string detail() const = 0;
+  /// Descriptions of the phases that are expected in the future
+  virtual std::vector<Phase::Pending> pending_phases() const = 0;
 
-  /// The original task Request that spawned this Task
-  virtual const Request& original_request() const = 0;
+  /// The request tag of this Task
+  virtual const Request::ConstTagPtr& tag() const = 0;
 
   /// Estimate the overall finishing time of the task
   virtual rmf_traffic::Time estimate_finish_time() const = 0;
 
   /// The Resume class keeps track of when the Task is allowed to Resume.
-  /// You can either call the object's operator() or let the object expire to
-  /// tell the Task that it may resume.
-  class Resume
-  {
-  public:
-
-    /// Call this object to tell the Task to resume.
-    void operator()() const;
-
-    class Implementation;
-  private:
-    rmf_utils::unique_impl_ptr<Implementation> _pimpl;
-  };
+  /// You can either call the Resume object's operator() or let the object
+  /// expire to tell the Task that it may resume.
+  class Resume : public detail::Resume {};
 
   /// Tell this Task that it needs to be interrupted. An interruption means
   /// the robot may be commanded to do other tasks before this task resumes.
@@ -107,10 +112,11 @@ class Task
 
   /// Kill this Task. The behavior that follows a kill will vary between
   /// different Tasks, but generally it means that the robot should be returned
-  /// to safe idle state as soon as possible, even if it remains encumbered by
+  /// to a safe idle state as soon as possible, even if it remains encumbered by
   /// something related to this Task.
   ///
-  /// The finished callback will be triggered when the Task is fully killed.
+  /// The Task should continue to be tracked as normal. When its finished
+  /// callback is triggered, the killing is complete.
   ///
   /// The kill() command supersedes the cancel() command. Calling cancel() after
   /// calling kill() will have no effect.

rmf_task/include/rmf_task/execute/TaskFactory.hpp

@@ -57,7 +57,7 @@ class TaskFactory
   using Activate =
     std::function<
     execute::ConstTaskPtr(
-      const Request& request,
+      const Request::ConstTagPtr& request,
       const Description& description,
       std::function<void(ConstConditionPtr)> update,
       std::function<void(ConstConditionPtr)> finished)