frc-2020's Introduction
frc-2020's People
frc-2020's Issues
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
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.
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
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.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.