Skip to content

Commit 48ece75

Browse files
committed
feat(updates): expose phase as x-medkit-phase vendor extension
SOVD UpdateStatus enum collapses prepare-completed and execute-completed into a single 'completed' value, leaving clients unable to tell whether the package is ready for Execute or fully installed. Surface the existing internal UpdatePhase as a vendor extension on /updates/{id}/status so consumers can distinguish the two without breaking the standard enum. - Move UpdatePhase into UpdateStatusInfo and emit it as `x-medkit-phase` - Keep the standard `status` field unchanged (pending|inProgress|completed|failed) - Document the extension in docs/api/rest.rst with a non-standard callout - Unit tests for prepared, executed, and freshly-registered (none) serialization
1 parent 72ae9b8 commit 48ece75

5 files changed

Lines changed: 90 additions & 34 deletions

File tree

docs/api/rest.rst

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1134,6 +1134,7 @@ Without such a plugin, all endpoints return ``501 Not Implemented``.
11341134
11351135
{
11361136
"status": "inProgress",
1137+
"x-medkit-phase": "preparing",
11371138
"progress": 65,
11381139
"sub_progress": [
11391140
{"name": "download", "progress": 100},
@@ -1147,6 +1148,13 @@ Without such a plugin, all endpoints return ``501 Not Implemented``.
11471148
so this endpoint returns ``200`` with ``{"status": "pending"}`` immediately after
11481149
registration, before any ``prepare`` or ``execute`` call.
11491150

1151+
**Vendor extension ``x-medkit-phase``** (non-standard, SOVD-compatible):
1152+
``none``, ``preparing``, ``prepared``, ``executing``, ``executed``,
1153+
``failed``, ``deleting``. Differentiates "prepare completed" (``status``
1154+
``completed`` + ``x-medkit-phase`` ``prepared``) from "execute completed"
1155+
(``status`` ``completed`` + ``x-medkit-phase`` ``executed``). Clients that
1156+
only consume the standard ``status`` field continue to work unchanged.
1157+
11501158
When ``status`` is ``failed``, an ``error`` object is included:
11511159

11521160
.. code-block:: json

src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_manager.hpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,6 @@ class UpdateManager {
101101
void notify_status_change(const std::string & id, const UpdateStatusInfo & status);
102102

103103
struct PackageState {
104-
UpdatePhase phase = UpdatePhase::None;
105104
UpdateStatusInfo status;
106105
std::future<void> active_task;
107106
};

src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_types.hpp

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,14 @@ struct UpdateFilter {
3030
std::optional<std::string> target_version; // Filter by target version
3131
};
3232

33-
/// Status of an update operation
33+
/// Status of an update operation (SOVD-compliant enum values)
3434
enum class UpdateStatus { Pending, InProgress, Completed, Failed };
3535

36+
/// Lifecycle phase - exposed as SOVD vendor extension `x-medkit-phase`.
37+
/// Differentiates "prepare completed" from "execute completed" which share
38+
/// status=completed in the SOVD standard enum.
39+
enum class UpdatePhase { None, Preparing, Prepared, Executing, Executed, Failed, Deleting };
40+
3641
/// Detailed progress for a sub-step of an update operation
3742
struct UpdateSubProgress {
3843
std::string name;
@@ -42,14 +47,12 @@ struct UpdateSubProgress {
4247
/// Full status information for an update operation
4348
struct UpdateStatusInfo {
4449
UpdateStatus status = UpdateStatus::Pending;
50+
UpdatePhase phase = UpdatePhase::None;
4551
std::optional<int> progress; // 0-100
4652
std::optional<std::vector<UpdateSubProgress>> sub_progress; // Detailed per-step progress
4753
std::optional<std::string> error_message; // Set when status == Failed
4854
};
4955

50-
/// Internal phase tracking for update lifecycle
51-
enum class UpdatePhase { None, Preparing, Prepared, Executing, Executed, Failed, Deleting };
52-
5356
/// Error codes for backend return values
5457
enum class UpdateBackendError {
5558
NotFound, // Package does not exist
@@ -91,7 +94,30 @@ class UpdateProgressReporter {
9194
std::mutex & mutex_;
9295
};
9396

94-
/// Serialize UpdateStatusInfo to SOVD-compliant JSON
97+
/// Serialize UpdatePhase to its `x-medkit-phase` string value.
98+
inline const char * update_phase_to_string(UpdatePhase phase) {
99+
switch (phase) {
100+
case UpdatePhase::None:
101+
return "none";
102+
case UpdatePhase::Preparing:
103+
return "preparing";
104+
case UpdatePhase::Prepared:
105+
return "prepared";
106+
case UpdatePhase::Executing:
107+
return "executing";
108+
case UpdatePhase::Executed:
109+
return "executed";
110+
case UpdatePhase::Failed:
111+
return "failed";
112+
case UpdatePhase::Deleting:
113+
return "deleting";
114+
}
115+
return "none";
116+
}
117+
118+
/// Serialize UpdateStatusInfo to SOVD-compliant JSON.
119+
/// Adds vendor extension `x-medkit-phase` to distinguish prepare-completed from
120+
/// execute-completed (both report SOVD status=completed).
95121
inline nlohmann::json update_status_to_json(const UpdateStatusInfo & status) {
96122
nlohmann::json j;
97123
switch (status.status) {
@@ -108,6 +134,7 @@ inline nlohmann::json update_status_to_json(const UpdateStatusInfo & status) {
108134
j["status"] = "failed";
109135
break;
110136
}
137+
j["x-medkit-phase"] = update_phase_to_string(status.phase);
111138
if (status.progress.has_value()) {
112139
j["progress"] = *status.progress;
113140
}

src/ros2_medkit_gateway/src/updates/update_manager.cpp

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,8 @@ tl::expected<void, UpdateError> UpdateManager::register_update(const nlohmann::j
113113
auto & state_ptr = states_[id];
114114
if (!state_ptr) {
115115
state_ptr = std::make_unique<PackageState>();
116-
state_ptr->phase = UpdatePhase::None;
117-
state_ptr->status = UpdateStatusInfo{UpdateStatus::Pending, std::nullopt, std::nullopt, std::nullopt};
116+
state_ptr->status =
117+
UpdateStatusInfo{UpdateStatus::Pending, UpdatePhase::None, std::nullopt, std::nullopt, std::nullopt};
118118
}
119119
return {};
120120
}
@@ -139,11 +139,11 @@ tl::expected<void, UpdateError> UpdateManager::delete_update(const std::string &
139139
it->second->active_task.wait();
140140
}
141141
// Mark as deleting so no new operations can start on this package
142-
it->second->phase = UpdatePhase::Deleting;
142+
it->second->status.phase = UpdatePhase::Deleting;
143143
} else {
144144
// Create sentinel to prevent concurrent start_prepare
145145
states_[id] = std::make_unique<PackageState>();
146-
states_[id]->phase = UpdatePhase::Deleting;
146+
states_[id]->status.phase = UpdatePhase::Deleting;
147147
}
148148
}
149149

@@ -157,7 +157,7 @@ tl::expected<void, UpdateError> UpdateManager::delete_update(const std::string &
157157
if (had_state) {
158158
auto it = states_.find(id);
159159
if (it != states_.end() && it->second) {
160-
it->second->phase = UpdatePhase::Failed;
160+
it->second->status.phase = UpdatePhase::Failed;
161161
}
162162
} else {
163163
// Remove the sentinel we created - package never had state before
@@ -203,12 +203,12 @@ tl::expected<void, UpdateError> UpdateManager::start_prepare(const std::string &
203203
state_ptr = std::make_unique<PackageState>();
204204
}
205205

206-
if (state_ptr->phase == UpdatePhase::Deleting) {
206+
if (state_ptr->status.phase == UpdatePhase::Deleting) {
207207
return tl::make_unexpected(UpdateError{UpdateErrorCode::Deleting, "Package is being deleted"});
208208
}
209209

210-
state_ptr->phase = UpdatePhase::Preparing;
211-
state_ptr->status = UpdateStatusInfo{UpdateStatus::Pending, std::nullopt, std::nullopt, std::nullopt};
210+
state_ptr->status =
211+
UpdateStatusInfo{UpdateStatus::Pending, UpdatePhase::Preparing, std::nullopt, std::nullopt, std::nullopt};
212212
state_ptr->active_task = std::async(std::launch::async, &UpdateManager::run_prepare, this, id);
213213
return {};
214214
}
@@ -229,7 +229,7 @@ tl::expected<void, UpdateError> UpdateManager::start_execute(const std::string &
229229
}
230230

231231
auto it = states_.find(id);
232-
if (it == states_.end() || !it->second || it->second->phase != UpdatePhase::Prepared) {
232+
if (it == states_.end() || !it->second || it->second->status.phase != UpdatePhase::Prepared) {
233233
return tl::make_unexpected(UpdateError{UpdateErrorCode::NotPrepared, "Package must be prepared before execution"});
234234
}
235235
if (is_task_active(id)) {
@@ -238,8 +238,8 @@ tl::expected<void, UpdateError> UpdateManager::start_execute(const std::string &
238238
}
239239

240240
auto & state = *it->second;
241-
state.phase = UpdatePhase::Executing;
242-
state.status = UpdateStatusInfo{UpdateStatus::Pending, std::nullopt, std::nullopt, std::nullopt};
241+
state.status =
242+
UpdateStatusInfo{UpdateStatus::Pending, UpdatePhase::Executing, std::nullopt, std::nullopt, std::nullopt};
243243
state.active_task = std::async(std::launch::async, &UpdateManager::run_execute, this, id);
244244
return {};
245245
}
@@ -273,12 +273,12 @@ tl::expected<void, UpdateError> UpdateManager::start_automated(const std::string
273273
state_ptr = std::make_unique<PackageState>();
274274
}
275275

276-
if (state_ptr->phase == UpdatePhase::Deleting) {
276+
if (state_ptr->status.phase == UpdatePhase::Deleting) {
277277
return tl::make_unexpected(UpdateError{UpdateErrorCode::Deleting, "Package is being deleted"});
278278
}
279279

280-
state_ptr->phase = UpdatePhase::Preparing;
281-
state_ptr->status = UpdateStatusInfo{UpdateStatus::Pending, std::nullopt, std::nullopt, std::nullopt};
280+
state_ptr->status =
281+
UpdateStatusInfo{UpdateStatus::Pending, UpdatePhase::Preparing, std::nullopt, std::nullopt, std::nullopt};
282282
state_ptr->active_task = std::async(std::launch::async, &UpdateManager::run_automated, this, id);
283283
return {};
284284
}
@@ -321,11 +321,11 @@ void UpdateManager::run_prepare(const std::string & id) {
321321
std::lock_guard<std::mutex> lock(mutex_);
322322
if (result) {
323323
state->status.status = UpdateStatus::Completed;
324-
state->phase = UpdatePhase::Prepared;
324+
state->status.phase = UpdatePhase::Prepared;
325325
} else {
326326
state->status.status = UpdateStatus::Failed;
327327
state->status.error_message = result.error().message;
328-
state->phase = UpdatePhase::Failed;
328+
state->status.phase = UpdatePhase::Failed;
329329
}
330330
snapshot = state->status;
331331
}
@@ -338,7 +338,7 @@ void UpdateManager::run_prepare(const std::string & id) {
338338
if (it != states_.end() && it->second) {
339339
it->second->status.status = UpdateStatus::Failed;
340340
it->second->status.error_message = std::string("Exception: ") + e.what();
341-
it->second->phase = UpdatePhase::Failed;
341+
it->second->status.phase = UpdatePhase::Failed;
342342
snapshot = it->second->status;
343343
}
344344
}
@@ -351,7 +351,7 @@ void UpdateManager::run_prepare(const std::string & id) {
351351
if (it != states_.end() && it->second) {
352352
it->second->status.status = UpdateStatus::Failed;
353353
it->second->status.error_message = "Unknown exception during prepare";
354-
it->second->phase = UpdatePhase::Failed;
354+
it->second->status.phase = UpdatePhase::Failed;
355355
snapshot = it->second->status;
356356
}
357357
}
@@ -377,11 +377,11 @@ void UpdateManager::run_execute(const std::string & id) {
377377
std::lock_guard<std::mutex> lock(mutex_);
378378
if (result) {
379379
state->status.status = UpdateStatus::Completed;
380-
state->phase = UpdatePhase::Executed;
380+
state->status.phase = UpdatePhase::Executed;
381381
} else {
382382
state->status.status = UpdateStatus::Failed;
383383
state->status.error_message = result.error().message;
384-
state->phase = UpdatePhase::Failed;
384+
state->status.phase = UpdatePhase::Failed;
385385
}
386386
snapshot = state->status;
387387
}
@@ -394,7 +394,7 @@ void UpdateManager::run_execute(const std::string & id) {
394394
if (it != states_.end() && it->second) {
395395
it->second->status.status = UpdateStatus::Failed;
396396
it->second->status.error_message = std::string("Exception: ") + e.what();
397-
it->second->phase = UpdatePhase::Failed;
397+
it->second->status.phase = UpdatePhase::Failed;
398398
snapshot = it->second->status;
399399
}
400400
}
@@ -407,7 +407,7 @@ void UpdateManager::run_execute(const std::string & id) {
407407
if (it != states_.end() && it->second) {
408408
it->second->status.status = UpdateStatus::Failed;
409409
it->second->status.error_message = "Unknown exception during execute";
410-
it->second->phase = UpdatePhase::Failed;
410+
it->second->status.phase = UpdatePhase::Failed;
411411
snapshot = it->second->status;
412412
}
413413
}
@@ -435,7 +435,7 @@ void UpdateManager::run_automated(const std::string & id) {
435435
std::lock_guard<std::mutex> lock(mutex_);
436436
state->status.status = UpdateStatus::Failed;
437437
state->status.error_message = prep_result.error().message;
438-
state->phase = UpdatePhase::Failed;
438+
state->status.phase = UpdatePhase::Failed;
439439
snapshot = state->status;
440440
}
441441
notify_status_change(id, snapshot);
@@ -445,7 +445,7 @@ void UpdateManager::run_automated(const std::string & id) {
445445
// Phase 2: Execute (reset progress for execute phase)
446446
{
447447
std::lock_guard<std::mutex> lock(mutex_);
448-
state->phase = UpdatePhase::Executing;
448+
state->status.phase = UpdatePhase::Executing;
449449
state->status.progress = std::nullopt;
450450
state->status.sub_progress = std::nullopt;
451451
}
@@ -457,11 +457,11 @@ void UpdateManager::run_automated(const std::string & id) {
457457
std::lock_guard<std::mutex> lock(mutex_);
458458
if (exec_result) {
459459
state->status.status = UpdateStatus::Completed;
460-
state->phase = UpdatePhase::Executed;
460+
state->status.phase = UpdatePhase::Executed;
461461
} else {
462462
state->status.status = UpdateStatus::Failed;
463463
state->status.error_message = exec_result.error().message;
464-
state->phase = UpdatePhase::Failed;
464+
state->status.phase = UpdatePhase::Failed;
465465
}
466466
snapshot = state->status;
467467
}
@@ -474,7 +474,7 @@ void UpdateManager::run_automated(const std::string & id) {
474474
if (it != states_.end() && it->second) {
475475
it->second->status.status = UpdateStatus::Failed;
476476
it->second->status.error_message = std::string("Exception: ") + e.what();
477-
it->second->phase = UpdatePhase::Failed;
477+
it->second->status.phase = UpdatePhase::Failed;
478478
snapshot = it->second->status;
479479
}
480480
}
@@ -487,7 +487,7 @@ void UpdateManager::run_automated(const std::string & id) {
487487
if (it != states_.end() && it->second) {
488488
it->second->status.status = UpdateStatus::Failed;
489489
it->second->status.error_message = "Unknown exception during automated update";
490-
it->second->phase = UpdatePhase::Failed;
490+
it->second->status.phase = UpdatePhase::Failed;
491491
snapshot = it->second->status;
492492
}
493493
}

src/ros2_medkit_gateway/test/test_update_manager.cpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -599,3 +599,25 @@ TEST(UpdateManagerFailureTest, ExecuteExceptionSetsFailedStatus) {
599599
manager.reset();
600600
backend.reset();
601601
}
602+
603+
TEST(UpdateStatusToJson, SerializesPhaseAsVendorExtension) {
604+
UpdateStatusInfo status{UpdateStatus::Completed, UpdatePhase::Prepared, std::nullopt, std::nullopt, std::nullopt};
605+
auto j = update_status_to_json(status);
606+
EXPECT_EQ(j["status"], "completed");
607+
EXPECT_EQ(j["x-medkit-phase"], "prepared");
608+
}
609+
610+
TEST(UpdateStatusToJson, ExposesExecutedPhaseForTerminalCompletion) {
611+
UpdateStatusInfo status{UpdateStatus::Completed, UpdatePhase::Executed, 100, std::nullopt, std::nullopt};
612+
auto j = update_status_to_json(status);
613+
EXPECT_EQ(j["status"], "completed");
614+
EXPECT_EQ(j["x-medkit-phase"], "executed");
615+
EXPECT_EQ(j["progress"], 100);
616+
}
617+
618+
TEST(UpdateStatusToJson, EmitsNonePhaseForFreshlyRegistered) {
619+
UpdateStatusInfo status;
620+
auto j = update_status_to_json(status);
621+
EXPECT_EQ(j["status"], "pending");
622+
EXPECT_EQ(j["x-medkit-phase"], "none");
623+
}

0 commit comments

Comments
 (0)