search

BrewPi / firmware

Brewing temperature control firmware for the BrewPi Spark (Particle Photon inside)
http://www.brewpi.com
GNU Affero General Public License v3.0
97 stars 55 forks source link

Feature/lib test separation #48

Closed elcojacobs closed 8 years ago

elcojacobs commented 8 years ago

This PR splits the tests into 2 separate builds.

I moved the test runner and dependencies to platform/test.

Instead of adding source files manually, I now include all of them with a wildcard search. This exposed some unnecessary includes (which I removed) and some build errors (which I fixed). It basically showed when lib and app was not properly separated and app files (like json files) where included in the lib sources.

I also had to add a logger.cpp file that did not depend on PiLink, because of log messages being generated in the OneWireTempSensor code.

This bit helps by printing a variable used in the makefile (for debugging makefiles): https://github.com/BrewPi/firmware/compare/develop...feature/lib-test-separation#diff-d930c5c4298ab4c8df1aa35edea8aac5R118

I changed the test tolerance from 0.5 to 1 in this bit because the difference was 0.51 on one occasion. With the random intervals, this can apparently happen. https://github.com/BrewPi/firmware/compare/develop...feature/lib-test-separation#diff-2f6f7edfb7b3c162953f23a6c9f30e3eL152

I think the file name ControllerMixins.h might be better renamed to libMixins.h, agree?

m-mcgowan commented 8 years ago

Agreed. Nicely done.

I might have called platform/test something like platform/gcc since it's the target platform rather than the function we need to describe. This might also be useful for compiling application code against gcc and not just test code.

elcojacobs commented 8 years ago

I called it platform/test, because it includes the boost test runner. By including that dir in the build, the various test dirs only have to hold the tests themselves.

elcojacobs commented 8 years ago

As discussed on hangouts: ActuatorDriver/ActuatorBottom contain application specific features that should move to the mixins. In ControlBox, the functions to find the last (bottom) actuator in a chain will not be needed. In BrewPi 0.4, they are only used by the device manager.

elcojacobs commented 8 years ago

I think this is done. Library should be pretty app neutral now. @m-mcgowan, please review the changes and give the go ahead to merge into develop.

m-mcgowan commented 8 years ago

I skim read it - didn't see any reason not to merge into develop. Some good refactoring there.

On Fri, Feb 26, 2016 at 2:28 AM, Elco Jacobs notifications@github.com wrote:

I think this is done. Library should be pretty app neutral now. @m-mcgowan https://github.com/m-mcgowan, please review the changes and give the go ahead to merge into develop.

— Reply to this email directly or view it on GitHub https://github.com/BrewPi/firmware/pull/48#issuecomment-189071855.