Team-OKC-Robotics / FRC-2022-CPP

a c++ port of our FRC-2022 code as training in C++
Other
0 stars 2 forks source link

Revisit software architecture design decisions #33

Open jkleiber opened 1 year ago

jkleiber commented 1 year ago

Summary In the port + improvement we made a lot of design decisions re: software architecture. It would be good to take a look at these decisions and make sure they were the right ones.

Specifically, I'd like to consider our stance on:

Work to Do

jkleiber commented 1 year ago

My initial perspective is the following:

  1. Hardware interfaces were a good idea in theory, but they do require much more code writing and passing pointers around than if the IO classes handled their own hardware. Additionally, the benefit is small since subsystems should not compete for the same actuator. However, the benefit could be high if two subsystems want to use the same sensor. Furthermore, I think the centralized list of motors is easy to read.
  2. Our parameter management system is largely untested and unproven. However, I think we should have one, so I think we should work hard to prove that TOML++ is the right or wrong answer.
  3. The global pointers we have for managing shuffleboard entries is a bad design, but that's the WPILib's fault for using references originally. In the 2023 code we should maintain those pointers in the appropriate software subsystem, and only have global ones for global things (competition mode, write mode)
  4. My opinion is that commands should always finish immediately (isFinished() always returns true). This reduces complexity, and we can cancel the command using button bindings. Generally our subsystems should be "set and forget" rather than relying on commands to actively manage the robot state.
  5. I think we should strive for the parameterized autonomous thing that reads from a file rather than needing to recompile. Recompiling takes way too long, and being able to test and tune quickly is very important given how long we typically get to spend with the robot before competition
jkleiber commented 1 year ago

With respect to point 4 above, there are some commands that may seem to involve complexity that actually don't. For example, in 2015 Recycle Rush, my former team had a really complicated system for automatically stacking totes. However, all of this complexity was managed by the subsystem in its periodic method The stacking algorithm is simple: Elevator:

  1. Unclamp trashcan holder
  2. If a tote is detected, drive stacking elevator down to preset pickup height
  3. Lift stacking elevator up to preset holding height
  4. Re-clamp trashcan holder if stack is high enough for the trashcan to be clamped Intake:
  5. Engage intake arms
  6. Run intake in

But the constraints were complicated:

Therefore the periodic function (if this had been written in C++) for the elevator code would need to know:

All this to say, I think a StackCommand could be implemented such that it runs as long as the Stack button is held. isFinished can be true always because the subsystems independently work together.

But this is where point 1 comes in: If both subsystems need to know where the other one is in order to avoid collision (this robot needed that capability early in the season until those problems were designed out), then the Hardware interface is actually useful (for sensors). So it probably makes sense for the actuators to be local to their specific SubsystemIO, but the sensors to be in a Sensor interface that may be shared between SubsystemIOs

danielbrownmsm commented 1 year ago
  1. Yeah, the whole hardware/software interface thing we did is a good design, but unnecessarily complicated for what we're doing. A centralized list of motors is nice, but we can do that with less complexity than what we have now. In the future, if we want to experiment with some more simulation, then the framework will come in handy. We're not quite there yet though.
  2. Yes, TOML is a lot better than the Constants file we had last season. We could even develop a shortcut or something that deploys or retrieves it via SSH so we have a better constants tuning flow (or something like that). If it doesn't work out though, it shouldn't be too bad, as a constants file shouldn't be too hard to throw together really quickly.
  3. Yeah, shuffleboard's always been a pain in the butt. Whatever works, and doesn't make us overrun loops, works for me.
jkleiber commented 1 year ago

Simulation is a big consideration. Realistically, we probably will get limited support for simulated devices (like CANSparkMax). The intent of the SubsystemIO classes is to drive the real robot or the simulation, so I think it doesn't really matter if we move the actuators into those classes as long as we have a ProcessIO and a ProcessSimIO function. I still think it makes sense to keep the SensorInterface centralized in case subsystems want to share sensors, and it allows us to expand back out into this framework in the future, so maybe we just don't nuke it for now and only use it for sensors?

danielbrownmsm commented 1 year ago

Yeah, if it works

danielbrownmsm commented 1 year ago
  1. Yes, for tele-op at least. Autonomous would need actual isFinished() with actual logic
danielbrownmsm commented 1 year ago
  1. but of course, if we had this, we might not need isFinished() with actual logic? Not sure how this gets implemented though