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

DutyCycleEncoder rewrite is not documented as "Breaking" #62

Open
chauser opened this issue Dec 3, 2024 · 3 comments
Open

DutyCycleEncoder rewrite is not documented as "Breaking" #62

chauser opened this issue Dec 3, 2024 · 3 comments
Labels
bug Something isn't working documentation Improvements or additions to documentation

Comments

@chauser
Copy link

chauser commented Dec 3, 2024

Describe the bug
DutyCycleEncoder.getAbsolutePosition changed toDutyCycleEncoder.get but https://docs.wpilib.org/en/latest/docs/yearly-overview/yearly-changelog.html#new-for-2025 just says DutyCycleEncoder was rewritten. DutyCycleEncoderSim is also affected for both the get and set methods.

There may be a similar issue with AbsoluteEncoder (mentioned as rewritten on the same line of the change log).

Expected behavior
At a minimum, the change log should flag the breaking change. Maybe the project importer could make the change automatically? Is that safe? I.e., is the change as simple as changing the method name or are there other semantic considerations that need to be accounted for? If so, some guidance is needed.

@chauser chauser added the bug Something isn't working label Dec 3, 2024
@ThadHouse
Copy link
Member

The behavior changed enough where we'd probably rather have the break so users actually look at where they used that code. Rollover support was completely removed, as it was broken in hardware. Doing this let us add better mapping support, so getting the absolute value as expected should be easier.

@KangarooKoala
Copy link

Should we update the docs page to describe it as breaking, though? (And maybe move it closer to the top? I'm not familiar with the typical structure of the "New for ..." pages to know if there's already a particular order they're in)

@chauser
Copy link
Author

chauser commented Dec 3, 2024

OK, that seems reasonable. I think then there needs to be a description of what to look at in user code that might be broken. The documentation for get is : "Get the encoder value since the last reset. This is reported in rotations since the last reset." which seems incorrect -- there isn't a reset method, so what would that mean? And "rotations" suggests to me that rollover support might be present. I think the new behavior is "return the encoder position in the interval 0..1 rotation".

@sciencewhiz sciencewhiz added the documentation Improvements or additions to documentation label Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

4 participants