Skip to content

Commit 33c1fd6

Browse files
committed
fixed an edge case where detach did not obey the requested model name, added clearer loggin
Signed-off-by: Adarsh Karan Kesavadas Prasanth <adarshkaran01@gmail.com>
1 parent adf8da1 commit 33c1fd6

File tree

2 files changed

+67
-38
lines changed

2 files changed

+67
-38
lines changed

src/systems/dynamic_detachable_joint/DynamicDetachableJoint.cc

Lines changed: 63 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,9 @@ void DynamicDetachableJoint::PreUpdate(
222222
this->childLinkEntity, "fixed"}));
223223
this->attachRequested = false;
224224
this->isAttached = true;
225+
// Keep track of the attached pair for future validation
226+
this->attachedChildModelName = this->childModelName;
227+
this->attachedChildLinkName = this->childLinkName;
225228
this->PublishJointState(this->isAttached);
226229
gzdbg << "Attaching entity: " << this->detachableJointEntity
227230
<< std::endl;
@@ -250,7 +253,10 @@ void DynamicDetachableJoint::PreUpdate(
250253
this->detachableJointEntity = kNullEntity;
251254
this->detachRequested = false;
252255
this->isAttached = false;
256+
// Publish using the last known attached pair, then clear them.
253257
this->PublishJointState(this->isAttached);
258+
this->attachedChildModelName.clear();
259+
this->attachedChildLinkName.clear();
254260
}
255261
}
256262
}
@@ -275,50 +281,69 @@ bool DynamicDetachableJoint::OnServiceRequest(const gz::msgs::AttachDetachReques
275281
_res.set_message("Invalid request: command must be 'attach' or 'detach'.");
276282
return true;
277283
}
278-
// Set the child model and link names
279-
this->childModelName = _req.child_model_name();
280-
this->childLinkName = _req.child_link_name();
281284

282-
// If attach is requested
283-
if (_req.command() == "attach")
284-
{
285-
if (this->isAttached)
285+
// If attach is requested
286+
if (_req.command() == "attach")
287+
{
288+
if (this->isAttached)
289+
{
290+
_res.set_success(false);
291+
_res.set_message("Already attached to child model " + this->attachedChildModelName +
292+
" at link " + this->attachedChildLinkName + ".");
293+
gzdbg << "Already attached" << std::endl;
294+
return true;
295+
}
296+
297+
// set the child model and link names from the request
298+
this->childModelName = _req.child_model_name();
299+
this->childLinkName = _req.child_link_name();
300+
this->OnAttachRequest(msgs::Empty());
301+
_res.set_success(true);
302+
_res.set_message("Attached to child model " + this->childModelName +
303+
" at link " + this->childLinkName + ".");
304+
}
305+
306+
// If detach is requested
307+
else if (_req.command() == "detach")
308+
{
309+
if (!this->isAttached)
310+
{
311+
_res.set_success(false);
312+
_res.set_message(std::string("Detach request received for ")
313+
+ this->attachedChildModelName + "/" + this->attachedChildLinkName);
314+
gzdbg << "Already detached" << std::endl;
315+
return true;
316+
}
317+
318+
// Validate that the request matches what is actually attached.
319+
const auto &reqModel = _req.child_model_name();
320+
const auto &reqLink = _req.child_link_name();
321+
if (reqModel != this->attachedChildModelName ||
322+
reqLink != this->attachedChildLinkName)
286323
{
287324
_res.set_success(false);
288-
_res.set_message("Already attached to another object");
289-
gzdbg << "Already attached" << std::endl;
325+
_res.set_message(
326+
"Detach rejected: requested " + reqModel + "/" + reqLink +
327+
" but currently attached to " + this->attachedChildModelName + "/" +
328+
this->attachedChildLinkName + "."
329+
);
290330
return true;
291331
}
292332

293-
this->OnAttachRequest(msgs::Empty());
294-
_res.set_success(true);
295-
_res.set_message("Attach request received.");
296-
}
297-
298-
// If detach is requested
299-
else if (_req.command() == "detach")
300-
{
301-
if (!this->isAttached)
302-
{
303-
_res.set_success(false);
304-
_res.set_message("Already detached");
305-
gzdbg << "Already detached" << std::endl;
306-
return true;
307-
}
308-
309-
this->OnDetachRequest(msgs::Empty());
310-
_res.set_success(true);
311-
_res.set_message("Detach request processed successfully.");
312-
}
313-
314-
else
315-
{
316-
_res.set_success(false);
317-
_res.set_message("Invalid command. Use 'attach' or 'detach'.");
318-
return true;
319-
}
320-
return true;
321-
}
333+
this->OnDetachRequest(msgs::Empty());
334+
_res.set_success(true);
335+
_res.set_message("Detached from child model " + this->attachedChildModelName +
336+
" at link " + this->attachedChildLinkName + ".");
337+
}
338+
339+
else
340+
{
341+
_res.set_success(false);
342+
_res.set_message("Invalid command. Use 'attach' or 'detach'.");
343+
return true;
344+
}
345+
return true;
346+
}
322347

323348
//////////////////////////////////////////////////
324349
void DynamicDetachableJoint::OnAttachRequest(const msgs::Empty &)

src/systems/dynamic_detachable_joint/DynamicDetachableJoint.hh

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,10 @@ namespace systems
113113
/// \brief Name of attachment link in the child model
114114
private: std::string childLinkName;
115115

116+
/// \brief Variables to keep track of what is currently attached
117+
private: std::string attachedChildModelName;
118+
private: std::string attachedChildLinkName;
119+
116120
/// \brief Minimum distance to consider the child link as attached
117121
private: double defaultAttachDistance = 0.1;
118122

0 commit comments

Comments
 (0)