Git Product home page Git Product logo

frc-2020's Introduction

FRC_CODE_2020

frc-2020's People

Contributors

lolshoc avatar lincolnsand avatar max-niederman avatar gagnonsilas avatar

Stargazers

ORB avatar

Watchers

James Cloos avatar Kalani Sanidad avatar  avatar ORB avatar

frc-2020's Issues

ChopShop (FRC-166) Code Review

Code Organization:

Classes in the control library are missing documentation - what do they do?
Subsystems have missing or inaccurate comments (Shooter references a periodic function that doesn’t seem to exist)
Commands seem to do more than they should - DriveWithJoysticks also manages intake management, but should probably be two separate commands that are the default commands for each subsystem.
Generally, it’s frowned upon for classes to directly expose members (m_drivePID for instance). It means that other code can modify it without the class knowing. At the very least, should change those to be private members, and expose them via methods that return references to them.
It’s a good practice to make sure that each file includes exactly what it needs, no more and no less. This helps make sure that code builds consistently when you include header files in any order.
It is also a good idea to make sure that all subsystems have comments. Comments may be brief, but to the point.
For subsystems with little logic it may be easier to include function implementations directly in the header file. For example the functions in the climb subsystem are all one liners, and could have been included in the header.
Logic:
Running calculate() for a PID controller (home grown or WPIlib) multiple times in one command will cause the numbers to turn out differently. Suggest caching the value or using a “at setpoint” function.
Avoid console output in anything that runs frequently as this can have a significant performance impact on the code.

WPIlib Usage:

WPIlib has a PID controller built in, and with the 2020 release the controller has been simplified. You may be able to look into using that, to remove the need for maintaining your own.
Related, it might make more sense for each command that uses PID to have their own PID controller - WPilib has a PID Command wrapper built in that may prove useful.
It doesn’t seem like NetworkTableHandler needs to be a whole subsystem - its purpose can be served by putting the network table entries in the subsystems that use them.
If a command requires multiple subsystems, then when it runs it interrupts any commands currently running on that subsystem. This is a useful tool, but means that if you have a command that requires multiple subsystems it’ll interrupt all of them. This can lead to confusing timing issues.
You may want to use the WPILib Timer class instead of std::chrono. The timer class has helpers for getting elapsed time. https://first.wpi.edu/FRC/roborio/release/docs/cpp/classfrc2_1_1Timer.html

Readability:

Variable names are important, you did a good job of naming the class members, but some function variables have extremely short names. Using longer names makes it easier to come back to code later, as well as enabling others to read your code.

Functionality:

I really like how you broke the code into different parts, making it easier to read and work on. One thing I would suggest would be to add some type of averaging of the drive inputs for the controller, so that the robot would be less jerky and a lot more smooth. After some testing earlier in our season we saw that this was very beneficial to our drivers. This can be easily implemented by taking a rolling average of the drive inputs.

Team 3045 Code Review Party

Hello Team 3636,

 Our team have a less advanced software group so we may not have the best feedback. With that, here's our feedback:

 - Maybe use inline functions to compact code. On the other hand, the written out functions are very readable and your code overall is really well organized.
 - Your 2d map is really nice. 
 - Maybe add an autonomous option to shoot preloaded balls (unless I missed it in your code)

Overall, we really like your code.
Team 3045

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.