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

update REVThroughBoreDutyCycleEncoder so it gets enough readings to initialize properly #94

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

dejabot
Copy link
Contributor

@dejabot dejabot commented Mar 18, 2024

Description

  • set the ConnectedFrequencyThreshold for REVThroughBoreDutyCycleEncoder to the right value. this allows isConnected to return true only after the encoder has enough readings to initialize properly.
  • update Shoulder just to use isConnected vs its own initialization timer

How Has This Been Tested?

Needs testing on RevB

  • Unit tests: [Add your description here]
  • Simulator testing: [Add your description here]
  • On-robot bench testing: [Add your description here]
  • On-robot field testing: [Add your description here]

…r, so isConnected() returns when it's had enough readings to initialize
@@ -8,6 +8,7 @@ public class REVThroughBoreDutyCycleEncoder extends DutyCycleEncoder implements
public REVThroughBoreDutyCycleEncoder(int channel) {
super(channel);
setDutyCycleRange(1. / 1025., 1024. / 1025.);
setConnectedFrequencyThreshold(976 /* 975.6 on spec sheet*/);
Copy link
Member

Choose a reason for hiding this comment

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

public boolean isConnected() {
  return getFrequency() > m_frequencyThreshold;
}

so we might want to be careful about rounding up the threshold (especially since isConnected uses strict greater-than)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah, I see. at 976, it'll never "spin up".. nice catch. changing to 975.

Copy link
Member

Choose a reason for hiding this comment

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

please test this a significant number of times on-robot. i would like to make sure that it regularly reaches 976 Hz (or whether we should set the threshold frequency at 974 so that it will allow 975 Hz as well)

@dejabot
Copy link
Contributor Author

dejabot commented Mar 18, 2024

thank you! good catch on the strict >. changed threshold to 975.

PTAL?

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