Quote:
Originally Posted by joelg236
I did a diff on your CheezyGyro class, and the only important change was making initGyro() public. Is this only to calibrate the gyro when you want to? Why didn't you just adjust it in the wpilibj and compile with the custom version?
|
In industry, modifying the source of an external library is a bad practice that should be avoided if a better technical solution exists.
The bigger issue is if the other library receives updates from the original source. Then you have to re-implement the changes into the updates. If the custom change were in runtime logic (not detected by a compile error, like the Poof's would be) then your robot wouldn't act correctly, causing a lot of wasted time in trying to figure out why.
Here's an example of a situation where I was forced to modify COTS code at work. It required a full write-up and sign-off by the program manager. This type of thing isn't detected by a compiler, but it caused several runtime Exceptions when we tried to load the map format.
At work, we use NASA WorldWind. WorldWind has some static file name extensions that are hard-coded to lower case. On Linux, the files we have are upper case, and case matters on Linux. One of the map formats we use contains the upper-case file names in a header table embedded within a single file binary file -- not possible to change. Additionally, if the issue were brought up to NASA, the fix would likely be seen in WorldWind 1.5 or 1.6, whereas we're using 1.4. WorldWind 1.5, at the time the decision was made, was having hotspot issues on Linux -- so waiting for a fix in an update was considered a risk. Thus, we had to modify WorldWind's code (added .toUpperCase() in the proper spots). We then CM'ed the specific patch files so we would know exactly what changes had to be made in the future. Any time we want to update WorldWind (perhaps in the project's next iteration), we can pull the patch files out of the code repository and analyze if they're still relevant.