Skip to content
This repository has been archived by the owner on Jun 19, 2024. It is now read-only.

refactor(breaking): Motor API #66

Merged
merged 38 commits into from
Mar 13, 2024
Merged

refactor(breaking): Motor API #66

merged 38 commits into from
Mar 13, 2024

Conversation

Tropix126
Copy link
Member

Describe the changes this PR makes. Why should it be merged?

Refactors the Motor API.

  • Adjusts constructor arguments as defined in Rethink Motor constructor arguments. #49.
  • implements TryFrom for Gearset so that the bail_on is in the conversion rather than the getter
  • Adds support for getting brake modes
  • Renames get_state to state
  • Changes reversed to return Result<bool, _> rather than just false if PROS_ERR is returned

Additional Context

@Tropix126 Tropix126 mentioned this pull request Jan 19, 2024
Closed
@doinkythederp doinkythederp added semver:major Changes the existing API in a backward-incompatible manner type:enhancement New feature or request scope:smart_devices Updates smart devices support labels Jan 20, 2024
Gavin-Niederman
Gavin-Niederman suggested changes Feb 26, 2024
View reviewed changes
Copy link
Member

@Gavin-Niederman Gavin-Niederman left a comment

Choose a reason for hiding this comment

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

This looks great other than a couple missing docs.

packages/pros/src/devices/smart/motor.rs Outdated Show resolved Hide resolved
packages/pros/src/devices/smart/motor.rs Outdated Show resolved Hide resolved
@Tropix126
feat: (hopefully) full coverage of the PROS motor api
5f2c100
@Tropix126
chore: fmt
97f1f12
@Tropix126
docs: reword warning
c45a047
@Tropix126
docs: add comments for missing functions
12ec99d
@Tropix126
feat: set_profiled_velocity
42caf61
@Tropix126
refactor/docs: clean up MotorTuningConstants
1625a58
@Tropix126
refactor: various subjective API tweaks
5f4f173
@Tropix126
docs: document voltage_limit unit uncertainty
d522bca
@Tropix126
Merge remote-tracking branch 'upstream/main' into refactor/motor
391ed9c
@Tropix126
fix: dangerous_motor_tuning feature flag
68a7404
@Tropix126
refactor: rename MotorTarget to MotorControl
42d1614
@Tropix126
Merge branch 'main' into refactor/motor
427ba08
@Tropix126
refactor: make voltage_limit non-optional
c2f96c1
@Tropix126
Merge branch 'refactor/motor' of https://github.com/Tropix126/pros-rs …
9a0aedd 
…into refactor/motor
@Tropix126
refactor: use bitflags crate for status structs, deprecate MotorSta…
b6924b2 
…tus things
@Tropix126
refactor: InertialStatus bitflags
a88c9c4
@Tropix126
refactor: Direction enum
a70fa58
Gavin-Niederman
Gavin-Niederman suggested changes Mar 12, 2024
View reviewed changes
Copy link
Member

@Gavin-Niederman Gavin-Niederman left a comment

Choose a reason for hiding this comment

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

Just a few minor things I noticed. This should be merged soon.

CHANGELOG.md Outdated Show resolved Hide resolved
packages/pros-devices/src/smart/imu.rs Show resolved Hide resolved
Gavin-Niederman
Gavin-Niederman approved these changes Mar 13, 2024
View reviewed changes
Copy link
Member

@Gavin-Niederman Gavin-Niederman left a comment

Choose a reason for hiding this comment

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

LGTM

@Gavin-Niederman Gavin-Niederman merged commit 21f45c4 into vexide:main Mar 13, 2024
3 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ImpossibleReality ImpossibleReality ImpossibleReality approved these changes

@Gavin-Niederman Gavin-Niederman Gavin-Niederman approved these changes

Assignees
No one assigned
Labels
scope:smart_devices Updates smart devices support semver:major Changes the existing API in a backward-incompatible manner type:enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Rethink Motor constructor arguments.
4 participants
@Tropix126 @ImpossibleReality @Gavin-Niederman @doinkythederp