Skip to content

Implement manual mode arm API (freedrive) - #230

Draft
Nicolas Menard (npmenard) wants to merge 1 commit into
mainfrom
RSDK-14465-implement-manual-mode-api
Draft

Nicolas Menard (npmenard) wants to merge 1 commit into
mainfrom
RSDK-14465-implement-manual-mode-api

Conversation

@npmenard

Copy link
Copy Markdown
Contributor

Stack created with GitHub Stacks CLI • Give Feedback 💬

@npmenard
Nicolas Menard (npmenard) added this pull request to stack #231 September 11, 2026 13:45
@npmenard
Nicolas Menard (npmenard) force-pushed the RSDK-14465-implement-manual-mode-api branch from 0969308 to ea177c1 Compare September 11, 2026 14:04
@npmenard
Nicolas Menard (npmenard) marked this pull request as ready for review September 11, 2026 14:04
@npmenard
Nicolas Menard (npmenard) force-pushed the RSDK-14465-implement-manual-mode-api branch from ea177c1 to 00de455 Compare September 11, 2026 15:29
@npmenard
Nicolas Menard (npmenard) marked this pull request as draft September 11, 2026 15:29

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General feeling is that this is breaking the encapsulation of the state machine. Should freedrive just BE a state that the arm can be in?

return {/*support_manual_mode=*/false, /*support_cartesian_commands=*/true};
// Freedrive is UR's manual (gravity compensation) mode, and
// `move_to_position` provides direct cartesian commands.
return {/*support_manual_mode=*/true, /*support_cartesian_commands=*/true};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd drop the inline comments. This repo is C++20 so you can use designated initializer syntax here if you want to make it clear, probably:

return { .support_manual_mode. true, .support_cartesian_commands = true };

// If we are no longer in the controlled state, the control script has
// already left freedrive (or is gone entirely); clearing our tracking
// above is all there is to do.
if (auto* const controlled = std::get_if<state_controlled_>(&current_state_)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is more work, but the right way to do this is to delegate to the states.

// MODE_FORWARD message would knock the control script out of freedrive)
// and enforces `freedrive_deadline_`, the automatic exit time, when set.
bool freedrive_active_{false};
std::optional<std::chrono::steady_clock::time_point> freedrive_deadline_;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you not use the deadline to know whether you are active or not? Do you also need the bool?

}

void URArm::state_::send_noop_() {
if (handle_freedrive_()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes me think that freedrive should be its own state?

Base automatically changed from RSDK-14465-bump-viam-cpp-sdk-0.41 to main September 11, 2026 19:01
@npmenard
Nicolas Menard (npmenard) force-pushed the RSDK-14465-implement-manual-mode-api branch from 00de455 to ed83caf Compare September 11, 2026 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants