search

robotpy / robotpy-rev

RobotPy bindings for REV Robotics' REVLib
Other
8 stars 13 forks source link

Reduce the amount of wpilibc #3

Closed auscompgeek closed 5 years ago

virtuald commented 5 years ago

Did you actually try to compile this? Pretty sure that SpeedController.h is required for build to work.

virtuald commented 5 years ago

I've enabled travis testing similar to CTRE. Please rebase against master, thanks!

virtuald commented 5 years ago

I guess this is fine.

My opinion is that I'd rather copy the entirety of files instead of copying bits and pieces of files, as that would allow us in the future to just copy files wholesale instead of trying to figure out if any of the relevant pieces changed. Even with a few extra functions/files... it doesn't seem that it would really add much overhead.

I can be convinced otherwise. Why should we reduce the amount of wpilibc?

auscompgeek commented 5 years ago

When I started staring at this, I suppose I was mainly concerned about the surface area for potential linker breakage, especially with the unnamespaced functions. This point is probably moot though - ld.so resolves everything to the first matching symbol it finds, I think.

There is the point of lowering compile times, I suppose.

as that would allow us in the future to just copy files wholesale

This is a good point. On the other hand, one might consider the fact that taking bits and pieces from DriverStation is required anyway. It'd probably be worth slimming things down if we have to do that anyway.

I was slightly disappointed that I couldn't just delete Timer and Utility wholesale though (since as it turns out they're used by Error).

auscompgeek commented 5 years ago

Superceded by #7.