Discuss possible coding/codebase standards

[ZWORX52]

Topic ideas:

• naming: use m_? constants naming? networktables names?
• constants: what should be in constants.java (the global one) (tolerance)?
• formatting: use jdtls formatting versus hand-formatting? formatting styles? perhaps clang-formatting for better configuration?
• git stuff: when should we make a new branch? how fine should our commits be?
• commands: when to use factories versus a full command?
• documentation: how much? how much jdoc?

more could be added as an amendment later. please discuss!

[JacobyIA]

I think it may be good to–in the post season or during some downtime–get the software team together and discuss this in person, such that we may be more clear with our ideas and explanations. This will also be good as we never really have software meetings and there are other organization-related topics which need to be discussed.

[minglethepringle]

• Naming: wpilib stuff likes to use a lot of m_ and whatnot. Personally it makes it harder to read, I go camelCase for variables, SCREAMING_SNAKE_CASE for constants, and PascalCase for classes as usual. It's you who is gonna be modifying the code anyway
• Global constants: Highly recommend separating out constants into separate files. Different subsystems have different constants requirements
• Formatting: Just use normal indentation with tabs man, simple as that
• Git stuff: Keep main/master fully working at all times. Make a branch for individual features that you want to implement. Commit early and commit often, it only gives you benefits when you are able to revert to a previous commit
• Commands: Not sure what you mean by "full command", but if you're talking about Commands.sequence vs new SequentialCommandGroup for example, Commands.sequence is usually cleaner. They do the same thing
• Full javadocs is overkill, but uncommented code is unreadable. Just leave normal comments // in various places that are helpful

[ZWORX52]

I like your first point. I've started writing members without m due to reading 190's code, and I'll most likely continue after reading that. Constants: Yes. We should probably also remove the prefix (think ANGLE from e.g. ANGLE_START_POS_DEG) because we already reference them with AngleConstants.. Formatting: ... yes but jdtls if used correctly can also fix the problem and personally I prefer it. Git: Okay. I read this as "main should be a stable branch." Should I make a develop branch then? If so, I'll fast-forward all the topic branches. Commands: By full command I mean a file for each command vs factories. Okay, I'll use commands.sequence. Javadocs: I'm really bored okay? Re: commands: There are some requirement shenanigans because if a command run during auto requires a subsystem, the defaultcommand for that subsystem never runs, so some commands just don't require their subsystem (see AngleSubsystem.getSetAngle for example). Is that okay or should we try another approach?

[ZWORX52]

Also: how much should we spam topic branches? For example, I could make a branch to refactor out m_, but it would probably just have one commit and be merged immediately, so I could just as easily do it on main.

[ZWORX52]

Just in: I've made a develop branch and fast-forwarded the trapezoid and document branches up to it. Please develop off of it instead (i.e. make sure to git checkout develop before git branch <topic> or git checkout -b <topic>). Topics off of topics are allowed.

[minglethepringle]

Formatting: doesn't matter what tech you use, just gotta make sure things are indented properly. There's only one correct way to indent Java code! Git: no need, could just develop on branches and then merge to main in a PR. I saw you already created develop though, no worries Commands: separate out "unit" commands e.g. WristSetAngle to separate files. Composition commands e.g. with sequence can go into one big file like WristSubsystemCommands Javadocs: go ahead lol Re:commands: sounds like the wrong approach. With a wrist for example, you would want it to always be running the "go to angle" command and have it never stop. When you change the angle, you don't want a whole command for that, but rather a command (maybe using runOnce) to just change the setpoint for the wrist, which is just one method call. So nothing would ever require the wrist subsystem and it'll just change the setpoint and the constantly running command will then go to the new setpoint

[JacobyIA]

So I think that there is some misunderstanding re:commands, we have a constantly running, subsystem required PIDCommand, in addition to a subsystem un-required set setpoint command.

[minglethepringle]

The set setpoint command does not need to require the subsystem because the command is not controlling motors or sending conflicting signals with the pidcommand (which i hope incorporates feedforward too).

[JacobyIA]

Right, I believe thats what I said

[minglethepringle]

Oops, didn't read carefully enough, you're right. I haven't read y'all code so my bad

[JacobyIA]

Saul Good, Man

[JacobyIA]

I have been thinking on it and looking at other codebases, and I believe that we should get on the Enumeration train. Enums would be useful for global states (though networktables could be used also) as well as subsystem goals for logging purposes. Additionally, they would provide for much more readable code.

[minglethepringle]

Don't use network tables as a source of truth for state, you'll have issues of network latency and race conditions.I definitely recommend enums for different stateful positions and having individual subsystems contain the source of truth of state

[JacobyIA]

That is exactly my fear (it is concerning how often we agree like this)

[minglethepringle]

Feel free to hit me up on discord to bounce any ideas off me! I've got 6 years of frc coding experience and counting 😀

[ZWORX52]

Okay so i'll throw in what I see re: commands again. From what I can tell, we only have a few "complex" commands, i.e. those that can't be solved with a runOnce or would look very ugly that way. Those I've chosen to keep as full .java files (you can see them in the commands/ folder) All other commands are factories of some sort, with FactoryCommands aiding in the more annoying syntaxes (I hate the BooleanSupplier inline subclassing nonsense) Aside from that, the only other commands we have are compositions, in Compositions.java.

Is such an architecture what you're thinking about/recommending? Or am I factory-ifying too much?

[minglethepringle]

I'll take a look at the code when I can, overall don't worry about it because no matter how it's formatted, it works 😎

[ZWORX52]

The question right now is actually whether it works -- all of these changes are since the last competition, and I really only get 3-4 hours (today after school until the rest of the team comes) to test it all. Such is the life of software.

[ZWORX52]

Also -- good to see you're not nocturnal. ;)

[minglethepringle]

Good luck lol