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.

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

Updates to Servo class and fix ESP32 servo issue on beta 55 #372

Merged
merged 7 commits into from
Dec 1, 2022
Merged

Updates to Servo class and fix ESP32 servo issue on beta 55 #372

merged 7 commits into from
Dec 1, 2022

Conversation

brentru
Copy link
Member

@brentru brentru commented Nov 22, 2022

  • Automatically writes min pulse width value to servo object upon creation
    • This removes 1x protobuf data transfer, reduces bandwidth
    • @lorennorman - Please remove the default servo write on staging, let me know here when ready to test.
  • Adds getServoIdx for performing index lookup
  • Initialize servo struct with default values instead of init'ing within the constructor
  • Fixes servo write on ESP32x platforms by using the LEDC timer width from beta 54, instead of the 16bit width from beta 55.

@brentru brentru requested a review from lorennorman November 22, 2022 19:25
@brentru
Copy link
Member Author

brentru commented Nov 28, 2022

Artifacts are ready for testing + checks passed

Copy link
Contributor

@lorennorman lorennorman left a comment

Choose a reason for hiding this comment

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

Only necessary change is the misleading error message, the rest is me having fun.

src/components/servo/ws_servo.cpp Outdated Show resolved Hide resolved
src/components/servo/ws_servo.cpp Outdated Show resolved Hide resolved
src/components/servo/ws_servo.cpp Outdated Show resolved Hide resolved
src/components/servo/ws_servo.cpp Outdated Show resolved Hide resolved
@brentru brentru changed the title Updates to Servo class Updates to Servo class and fix ESP32 servo issue on beta 55 Dec 1, 2022
@brentru brentru requested a review from lorennorman December 1, 2022 19:51
Copy link
Contributor

@lorennorman lorennorman left a comment

Choose a reason for hiding this comment

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

💯

@brentru brentru merged commit 9091495 into adafruit:main Dec 1, 2022
@brentru brentru deleted the update-servo-class branch December 1, 2022 20:28
@lorennorman
Copy link
Contributor

@brentru ok this is live on staging for you to test!

@brentru
Copy link
Member Author

brentru commented Dec 5, 2022

@lorennorman Tested on hardware, OK'ing deploy to production


Prod

16:27:33.640 > ATTACHED servo w/minPulseWidth: 500 uS and maxPulseWidth: 500uS on pin: 27
16:27:33.648 > -> Servo Attach Response...Published!
16:27:33.917 > * NEW MESSAGE [Topic: Servo]: 
16:27:33.918 > 10 bytes.
16:27:33.918 > Decoding Servo Message...
16:27:33.924 > GOT: Servo Write
16:27:33.924 > Writing pulse width of 500uS to servo on pin#: 27
16:27:33.924 > 27
16:27:34.283 > PING!

Staging

16:32:11.316 > * NEW MESSAGE [Topic: Servo]: 
16:32:11.321 > 15 bytes.
16:32:11.321 > Decoding Servo Message...
16:32:11.327 > GOT: Servo Attach
16:32:11.327 > 27
16:32:11.327 > ATTACHED servo w/minPulseWidth: 500 uS and maxPulseWidth: 500uS on pin: 27
16:32:11.332 > -> Servo Attach Response...Published!

@lorennorman
Copy link
Contributor

@brentru I am wondering if this change has caused "sync" to not do the right thing now, can you test that easily?

To be clear: if there is already a value on the Servo's feed, the device should move to that position on sync (right?) I'm worried I broke this and need a further tweak.

@brentru
Copy link
Member Author

brentru commented Dec 6, 2022

Does not sync on staging

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