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

Added a TalonController to work with hardware-based PID control on Talon SRX devices #41

Merged
merged 6 commits into from
Dec 13, 2015

Conversation

rhauch
Copy link
Member

@rhauch rhauch commented Nov 16, 2015

Not yet ready for merging

Slightly modified the existing Controller interface (which is implemented for software-based PID control with PIDController) and added a TalonController that is able to configure, manage, monitor, and use PID control on the Talon SRX motor controller. The TalonController interface is quite large and complex, since it attempts to expose all of the wpilibj.CANTalon functionality without exposing any of the CANTalon class.

The new TalonController works with the existing ControlledCommand class that takes a Controller implementation.

A MockController class was also implemented to allow testing of components that use a Controller implementation, where the tests can manually set the error and verify the component responds correctly. This might be difficult for components that directly use PIDController or TalonController, but where possible the components should limit their use to Controller, which does allow changing the setpoint, tolerance, and enabling/disabling the controller. (This will not always be possible, since components may need to set or change PID gains.)

Closes #40

@rhauch
Copy link
Member Author

rhauch commented Nov 16, 2015

@granjef3, @Zabot: please review this PR and see if it sufficiently exposes the PID functionality on the Talon SRX.

@rhauch rhauch force-pushed the talon-pid-control branch 2 times, most recently from a55e3e7 to b13d50a Compare November 16, 2015 18:11
@rhauch
Copy link
Member Author

rhauch commented Nov 16, 2015

Updated the PR to reflect changes recently merged in #44, with a separate PIDController interface.

@rhauch rhauch force-pushed the talon-pid-control branch 3 times, most recently from 956e906 to 4ee3a49 Compare November 16, 2015 23:06
@rhauch
Copy link
Member Author

rhauch commented Nov 16, 2015

Changed TalonSRX fairly significantly:

  • Many of the methods that were in TalonController were not specific to feedback control, and thus were moved up to TalonSRX, which now has a lot of methods.
  • The methods that read the wired input sensors exposed a very small subset of the CANTalon functionality. Now, the quadrature encoder and analog input (pot or encoder) sensors can be read at any time on the TalonSRX, regardless of the mode of control. Both return the position (really an angle) and velocity (really a rate), so rather than expose individual AngleSensors and such, each is exposed as a single Gyroscope. This works really well, since the outputs are angles in degrees and rates in degrees per second, and are based upon the frame rates (which can be different for the encoder and analog sensor, and even changed by the user).
  • You can select which of these inputs is used as the "selected" input (even without using feedback control), in which case the corresponding input sensor is also exposed through a separate "selected" sensor Gyroscope. Here's the tricky part: the selected input sensor is sent from the Talon to the software at a much faster frame rate (by default 20ms rather than 100ms for the analog input and encoder sensors), so it's often much better and faster to select the desired input and then read it using the selected input rather than cranking up the frame rate.

The old TalonSRX interface was pretty small and introduced only 2 methods, but the old getAngleSensor():AngleSensor method was named poorly, so this PR deprecates the method and replaces it with getEncoderInput():Gyroscope. Existing code that uses Strongback 1.0.1 will work (since getAngleSensor() now just calls getEncoderInput(), but users will want to change to use the new method to get access to the rate.

@rhauch
Copy link
Member Author

rhauch commented Nov 16, 2015

@granjef3, @Zabot: If you don't have time to review, perhaps you could at least look at how HardwareTalonSRX is creating the gyroscope implementations, and specifically how it is converting from the analog bits or quadrature counts to angles and angular rates.

@xortive
Copy link

xortive commented Nov 17, 2015

I don't have thorough experience with the encoder functionality of the Talon SRX, but the gyroscope implementations look sane. Have you tested on real hardware yet?

@rhauch
Copy link
Member Author

rhauch commented Nov 17, 2015

@granjef3, thanks for scanning. No, this has not yet been tested on real hardware, though I hope to do that during the next week, at least with a quadrature encoder on our last robot. But we don't have an analog encoder or pot handy, so that will take a bit longer.

I realized that the target is exposed as raw position of whichever input sensor is selected, so I'll have another commit that will make it in degrees, and it will use the selected "enhanced" gyroscope (really, a special subclass of Gyroscope that can back-calculate the raw position or angles given a raw position). It'll also have unit tests for the "enhanced" gyro classes that use some examples from the Talon SRX Software Reference Manaual and the JavaDoc.

Also revamped how the input sensors are accessed via TalonSRX; both analog and quadrature encoder inputs are accessible via Gyroscope (since both position and velocity are available), and selecting one for feedback will be reflected in the separate selected input Gyroscope. The trick is that all 3 are available at the same time, although they operate upon different frames and at different rates.
…class. The unit tests verify the basic logic, but still don't match a real hardware test.
@rhauch
Copy link
Member Author

rhauch commented Dec 13, 2015

Rebased with latest from master.

rhauch added a commit that referenced this pull request Dec 13, 2015
Added a TalonController to work with hardware-based PID control on Talon SRX devices
@rhauch rhauch merged commit 338ad24 into strongback:master Dec 13, 2015
@rhauch rhauch deleted the talon-pid-control branch December 13, 2015 19:18
CarlinWilliamson pushed a commit to Team3132/strongback-java that referenced this pull request Jun 30, 2020
* Ready for testing. Forwards port 5802 to 22.

* Add rsync to constants and comment port forwading lines.

* Addressed comments in pull request.
CarlinWilliamson added a commit to Team3132/strongback-java that referenced this pull request Jun 30, 2020
* subsystem for buddyclimb added

* fixed constants and config

* Initial tweaks.

* everything should work except climber

* Fixed error after commit

* hopefully final changes to button mappings

* Added shooting sequence and nonblocking spin up.

* Implement waiting for shooter to come up to speed in controller.

* Minor rename from speed to rpm.

* +talonarraylists networktables, resolved comments

* Shortened getFMSColour()

* mock motor merge problems fixed

* Fixed comments from code review.

* did some diag mapping, cried a little, all g

* mapped some diag buttons (again), 2.0

* renamed several things to avoid confusion

* commented some code out

* removed talonArrayLists from motor factory

* fixed some issues with badly named things

* changed set in interfacetablehelper to get

* added sets,  execute(),  parse pidf

* missed commit

* replaced get()'s with set()'s tweaked comments

* Finish mapping buttons.

Some functionality had to be switched to toggle to match the button
design.

* resolved mark's comments

* tweaked indentation

* Tweaks to layout/comments.

* Change the source for the beam breaks.

* Add a comment.

* Cleaner way to indicate that the state doesn't care about the number of
balls or shooter should be up to speed.

This prevents a race condition where a ball is being ingested and the
state changes while it is being applied.

* Minor fixes.

Use spark max for passthrought and intake.
Plumb through the brake.

* LEDs countdown endgame and show alliance colours.

* Added functionality for setting charts to being at time = 0 every time a new chart is created.

* fixed issues found during robot testing

* Fix bug with last 15 seconds while in testing robot in teleop mode.

* Example OI based toggle mode.

Complicated way of implementing toggles in OI that handles running different
sequences based on the toggle state.

* More understandable way to toggle a boolean.

* intake/passthrough/loader/shooter testing

* Changed format of If statement.

* Shooter encoder now returns correct values.

* Shooter Tuning

* Changes from shooter testing.

* Loader and shooter tweaks to attempt to reduce
jamming.

* change intake from percentoutput to pid

* Fixed comments from pull request.

* Set graph log state to invalid every time logs are initialised to solve an issue where the csv header was overwritten with data

* Changed method name.

* Fixed manipulation of a double as a string

* intake/shooting sequence tweaks

* made suggested changes

* rotate image for vision

* intake/shooter sequence tweaks

* Fixing tests.

* Auto driving parameters from drivebase characterization.

* Fixes to get trajectory driving to work on falcons

* Fix motor scaling factor to make consistent.

* removed negatives from "paddleNotBlocking"

* changed rotate value 0

* Added basic auto routine to shoot, intake and drive

* reverted loader/shooter scale changes and tuned shooter

* adjust button mappings

* Make motor scaling more consistent, intuitive and
less error prone.

Implement a scaling function that takes encoder ticks, gearbox ratio
and if needed the number of metres per wheel turn.

Convert everything internally to rps instead of rpm and only show rpm on the
dashboard and logging.

* Spelling fail.

* implemented code suggestions and created hw subsystems to keep track of real subsystems

* fixed changes reverted by merge

* Convert everything from revs/min to revs/sec

This is more intuitive.

* Remove extrainious semi-colon

* Fixed RPM to RPS conversion

* fixes

* invert shooter motors

* inverted loader paddle pneumatic

* Vision fixes

* comments

* Better comment in getVisionDistance()

* comments

* Removed negatives from loader

* auto tweaks?

* resolved conflicts

* using startShooting sequence in auto

* updated comment

* edited sequence named

Co-Authored-By: CarlinWilliamson <31083798+CarlinWilliamson@users.noreply.github.com>

* Apply rotation to both images

* fixes

* Forwards port 5802 to 22. (strongback#41)

* Ready for testing. Forwards port 5802 to 22.

* Add rsync to constants and comment port forwading lines.

* Addressed comments in pull request.

* vision/intake tuning

* Added trench auto sequence

* changed names of constants to make more sense

* updated constant names - oops

* updated auto sequence to remove delays

* remove shooter delay

* reverting sequence change

* Logsync (strongback#52)

* Ready for testing. Forwards port 5802 to 22.

* Add rsync to constants and comment port forwading lines.

* can now read robot names

* Added reading of robot name and moving of logging
files.

* Added name detection and moved log files.

* Added latest and event logging usb folders.
Moved Latest_* files into the latest folder so they are the same depth
to make the html imports filesystem position agnostic.

* Create index.php

* Lognumber now does not materialize in sda1

* changed date format from ":" to "-"

* added runrsync to the utils folder

* deleted a single comment in run_rsync

* Update index.php

* fixed issue where logs were showing up in wrong
place due to the location in logdygraph being wrong

* Removed some debug lines, tidied up some imports
Made it work on windows

* got latest version of run_rsync

Co-authored-by: CookieCoder <cookiecoder15@gmail.com>
Co-authored-by: SebasPtsch <32213671+SebastianPietschner@users.noreply.github.com>
Co-authored-by: Amogh J <amoghj.au@gmail.com>

* First pass: Get a bunch of tests to work.

* All 44 unit tests now pass.

* Default the navx present to be the same as the drivebase.

* different shooter button mappings (strongback#50)

* different shooter button mappings

* better comments

* don't shoot at 0 RPS and show this state on the LEDS

* errors from merge

* OI changes from driver training

* Loader tuning after changing gearbox ratio

Co-authored-by: Hayley <hayl0830@gmail.com>

* renamed MotorOutput to DutyCycle (strongback#54)

* added maybewaitfor and time it took to apply state

* code review changes

* improved logging strings

* started logging for state

* Switch from toggle to mode based.

* Add back the toggle switch.

* Apply an endState when sequence is interrupted + sequence builder things (strongback#55)

* apply an end state if sequence gets interrupted

* fixes :D

* autofill interrupted state

* added unit test for interrupting the stop intaking sequence

* sequence builder

* fixes

* update sequences to use sequence builder

* fixed a null pointer + editted sequence names

* fixed a goof from merging earlier

* fix fillInterrupt() & javadocs

* updated what we auto fill in fillInterrupt()

* added 2nd constructor to sequenceBuilder

* edit: comment fix

Co-authored-by: carlinw <carlin.williamson1@gmail.com>

* added ledstrip to state and controller

* interrupts no longer put the drivebase back into arcade

* Changed buddyclimb in state.

* Rewrote buddy climb using onMode

* Removed random new line

* Changed from onMode to onToggle

* onToggle for climb/drive mode.

* Added maybe wait for buddy climb.

* Change names of methods to make more sense.

* Changed builder.add() to builder.then()

* logstring now logs

* added an actual method

* Move PID values to dedicated class. (strongback#61)

* Move PID values to dedicated class.

Also pass config instance to motor factory insted of lots of parameters.

* Rename symbol

* Clean up Motor interface and reclaim speed mode. (strongback#62)

* Move PID values to dedicated class.

Also pass config instance to motor factory insted of lots of parameters.

* Rename symbol

* Clean up Motor interface and reclaim speed mode.

Replace Speed with DutyCycle to make it clear that this isn't a speed.
Rename Velocity to be Speed since what the motor controllers call Velocity doesn't
have a direction (needed for Velocity).
Clean up the Motor interface to remove setSpeed() and only have
set(ControlMode, demand).

* Add a comment and remove checking if the target was found.

* added LED colours to the maybeWaits (strongback#60)

* add LEDs to maybeWaits + setAlternatingColour()

* updated the mock LED strip

* defining alternateNum using numberOfLEDs

* added a constant for LED %

* passing in debug colours into waitUntils

* changed pink to magenta & some fixes

* reworded controller waiting log message

* fix: alternating colours using the same colour

* Added auto/field positions to constants

* changed auto to use field locations

* Fixed up some constants

* Changed location to use 1st quadrant rather than 4th

* changed positions in constants to be pose2d

* Changed location origin to be center of the field

* Removed dygraph files (strongback#64)

* removed dygraph files

* removed dygraph from robot code

* helper methods for stopping at a distance using pose2d

* Removed Canifier related things + fixes

* more posehelper unit tests

* some more posehelper unit tests

* smaller posehelper tests

* formatting tab sizes

* fixed location ascii art :)

* removed most warnings (strongback#84)

* caching auto splines

* added createHash function

* unit tests

* Config server (strongback#82)

* Moved config server into robot code.

* Moved config server and passed in paths and port.

* Encodes and decodes html characters in config + testing

* Restart robot code when checkbox ticked.

* Wider input box on html and better test config.

* Stoped config server unit test from waiting.

* Addressed comments in pull request.

* Commented out 100 second wait in test.

* Removed onClick from checkbox.

* Refactored code inside of run method.

* test trajectories are now ignored by git, removed unnecessary stacktraces

* comments + moving voltage constraints to constants

* double checking creation/removal of trajectory files, removed unnecessary imports

* Add Robot Name to Config Server (strongback#90)

* Moved config server into robot code.

* Moved config server and passed in paths and port.

* Encodes and decodes html characters in config + testing

* Restart robot code when checkbox ticked.

* Wider input box on html and better test config.

* Stoped config server unit test from waiting.

* Addressed comments in pull request.

* Commented out 100 second wait in test.

* Removed onClick from checkbox.

* Refactored code inside of run method.

* Add robot name as a parameter in config webserver.

* Addressed comments in pull request.
Refactored code.

* Removed useless todo.

* Drivebase refactoring (strongback#89)

* DriveRoutine is now an abstract class

* Removed DriveMode

* added deadbands to arcade drive routines

* removed unnecessary this.

* removed outdated TODO/FIXME

* forcing cheesy to use duty cycle

* Fixed commented out code autocomplete

* Added low pass filter to vision.java, which may require changes to th… (strongback#91)

* Added low pass filter to vision.java, which may require changes to the alpha and timeOffset values in order to be tuned correctly

* Last seen target is validated using isValid, and alpha constant has been moved to Constants.java

* Commented out redundant height parameter in vision targeting

* Removed height parameter from vision.java (no worries carlin)

* Edited low pass filter to only filter x and y co-ordinates and heading of target location

* Removed unused import

* Added a test to check that we are reading from cached files as expected

* fails file reading test when exceptions are thrown

* updated gitignore - including a cached spline

* removed unused import

* removed some unmappable characters

* add timestampt to the begining of the event log filename

* fixes for typos and comma location

* added underscore to indicate symbolic link

* Calendar.getInstance to method

* Set some test config so hal stuff works.

* fixed java time layout vs php

* initial test of UTF-8

* github action for building and testing frc code

* testing no longer discriminates against linux users

* PR template

* revisions and spelling is hard

* added createPose2d to encourage using degrees not radians

* Don't normalise angles

* Move to a much simpler, static config class and remove using constants for defaults. (strongback#97)

* Beginnings of a more simple config file.

* Getting closer to unit testing and better config layout.

* Swap to Config from RobotConfiguration

Move more parameters to config so that it can be removed from Constants.
Put Constaints on a diet to remove redundant values.
Add ConfigReader unit test.
Stop passing config everywhere, using a static instance instead.
Use namespaces in Config class for better grouping and more consistency.

* Ensure example config is written out.

* Separate logging and charting.

Move to static methods so that they don't need to be passed around.

* Rename method to make it consistent

* Add unit test for TimestampedLogWriter.

* Move things around to reove a class as per pull request.

* Changes from reivew.

Renamed Logger->Log and other minor changes.

* Added issue templates

* Punctuation cause it bother's everyone

But you did complain didn't you Hayley

* Punctuation cause it bother's everyone

But you did complain didn't you Hayley

* Update .github/ISSUE_TEMPLATE/feature_request.md

* trajectory test files are now created in a temp directory

* test that we are actually reading from a cached file

* updated javadocs

* Move constants into Config class. (strongback#110)

* Beginnings of a more simple config file.

* Getting closer to unit testing and better config layout.

* Swap to Config from RobotConfiguration

Move more parameters to config so that it can be removed from Constants.
Put Constaints on a diet to remove redundant values.
Add ConfigReader unit test.
Stop passing config everywhere, using a static instance instead.
Use namespaces in Config class for better grouping and more consistency.

* Ensure example config is written out.

* Separate logging and charting.

Move to static methods so that they don't need to be passed around.

* Rename method to make it consistent

* Add unit test for TimestampedLogWriter.

* Move things around to reove a class as per pull request.

* Changes from reivew.

Renamed Logger->Log and other minor changes.

* Move constants into Config class.

* Merge fixes

* Remove redundant line.

* renamed subsystem interfaces

* renamed files for consistency

* more renames for consistency

* Renamed files for consistency

* removed strongback

* moved NetworkTableHelper implementations into strongback

Co-authored-by: Jamie Ha <jamieha9@gmail.com>
Co-authored-by: Abhishek <abhishek.kuchibhotla@gmail.com>
Co-authored-by: yakyakyakyak <jethro.rosette@outlook.com>
Co-authored-by: Mark Waldron <mark.r.waldron@gmail.com>
Co-authored-by: CookieCoder <cookiecoder15@gmail.com>
Co-authored-by: Amogh J <amoghj.au@gmail.com>
Co-authored-by: MrFuzzykins <shawn.huttonism@gmail.com>
Co-authored-by: jwjamesking <jkwainwright13@gmail.com>
Co-authored-by: Hayley <hayl0830@gmail.com>
Co-authored-by: Sebastian <sebastian.pietschner@gmail.com>
Co-authored-by: CookieCoder15 <40507205+CookieCoder15@users.noreply.github.com>
Co-authored-by: yakyakyakyak <56021544+yakyakyakyak@users.noreply.github.com>
Co-authored-by: SebasPtsch <32213671+SebastianPietschner@users.noreply.github.com>
Co-authored-by: idontknowhowtopeople <57520886+idontknowhowtopeople@users.noreply.github.com>
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