Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Arm Inverse Kinematics Updates #270

Merged
merged 17 commits into from
Jan 9, 2024
Merged

Arm Inverse Kinematics Updates #270

merged 17 commits into from
Jan 9, 2024

Conversation

Geeoon
Copy link
Member

@Geeoon Geeoon commented Oct 10, 2023

Changes setArmIKEnabled packet to requestArmIKEnabled.
Adds reportArmIKEnabled packet
No longer crashes if enabling arm IK doesn't succeed. Instead it sends a new packet, reportArmIKEnabled which will let MC know if arm IK is enabled or not.

@Geeoon Geeoon requested a review from abhaybd October 10, 2023 01:16
@Geeoon Geeoon self-assigned this Oct 10, 2023
abhaybd
abhaybd requested changes Oct 10, 2023
View reviewed changes
Copy link
Member

@abhaybd abhaybd left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Just one minor change needed, otherwise looks good.

Also, we should send a reportIkEnabled packet when mission control connects, since if IK is enabled we should tell them that

src/network/MissionControlProtocol.cpp Outdated Show resolved Hide resolved
@Geeoon Geeoon requested a review from abhaybd October 13, 2023 01:02
abhaybd
abhaybd requested changes Dec 14, 2023
View reviewed changes
Copy link
Member

@abhaybd abhaybd left a comment

Choose a reason for hiding this comment

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

Mostly good, just some minor nitpicks we should iron out

src/network/MissionControlProtocol.h Outdated Show resolved Hide resolved
src/network/MissionControlProtocol.cpp Outdated Show resolved Hide resolved
abhaybd
abhaybd previously approved these changes Jan 7, 2024
View reviewed changes
Copy link
Member

@abhaybd abhaybd left a comment

Choose a reason for hiding this comment

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

Looks good! Let's hold off on merging until the MC PR is ready, though.

@Geeoon Geeoon requested a review from abhaybd January 9, 2024 02:04
abhaybd
abhaybd approved these changes Jan 9, 2024
View reviewed changes
Copy link
Member

@abhaybd abhaybd left a comment

Choose a reason for hiding this comment

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

Looks good! Just waiting on the MC pr now.

@abhaybd abhaybd merged commit ec3cd15 into master Jan 9, 2024
3 checks passed
@abhaybd abhaybd deleted the ik-updates branch January 9, 2024 05:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abhaybd abhaybd abhaybd approved these changes

Assignees

@Geeoon Geeoon

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Geeoon @abhaybd