utra-robosoccer / soccer-embedded

Collection of embedded programs for an autonomous humanoid soccer robot
http://utrahumanoid.ca
32 stars 8 forks source link

Move includes to same folder as source files #185

Closed rfairley closed 5 years ago

rfairley commented 5 years ago

For now, this adds Common/component as an include path. I think however we should later change it so only Common is an include path, and components will be included in source files as #include "component/UartDriver/UartDriver.h", #include "interface/UartInterface.h", #include "mock/MockUartInterface.h", etc. This to me makes it clear what kind of thing is being included. There may be other opinions on this though. We can possibly discuss and make this edit here.

There are 5 files in Common/include that need to be split out still.

BufferBase.h
Notification.h
PeripheralInstances.h
SystemConf.h
uart_handler.h

BufferBase.h -> I'm thinking we could possibly split BufferMaster from BufferBase, as BufferMaster seems more application specific. Will defer to @gokuldharan on that Notification.h -> This code is generic and it seems like it could be part of a util namespace, as both things in app and component need it. Though, further decoupling might be needed elsewhere (as we don't necessarily want to enforce components using this file to have fixed values for e.g. NOTIFIED_FROM_TASK). It's a similar situation to SystemConf.h I'd say. SystemConf.h -> we'll track decoupling this in #177. PeripheralInstances.h, uart_handler.h -> we can deal with decoupling these and moving these to app while reworking BufferBase.h listed above.

I think it'd be good to sort out the BufferBase.h, PeripheralInstances.h, uart_handler.h files at the same time as this change. That'll get all our .h files living with their .cpp files. Leave the SystemConf.h and Notification.h for a separate issue.

Fixes #170

rfairley commented 5 years ago

@tygamvrelis and I discussed today and identified 2 other actionable items for splitting the includes mentioned above. Will create issues for these.

Lifted WIP - ready for review