Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.h splitting and reorg #780

Merged
merged 6 commits into from
Apr 11, 2018
Merged

.h splitting and reorg #780

merged 6 commits into from
Apr 11, 2018

Conversation

lobradov
Copy link
Contributor

  • Isolated dependencies
  • Sensors dependencies in individual sensor files.
  • enums, macros and prototypes.

Isolated dependencies

sensors dependencies

enums, macros and prototypes.

I2C dependencies moved to actual sensor files.

definitelly redefine
@@ -0,0 +1,76 @@
#ifndef DEPENDENCIES_H
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just as sensors this should be pragma once.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, changed.

@xoseperez
Copy link
Owner

It's a good approach, but I'm worried about the dependencies. For instance: modifying ADC_VCC_ENABLED is useless since this setting is used to set de ADC_MODE at the bottom of the sensors.h file... Maybe that call (ADC_MODE) should be in the analog sensor classes, same thing you did for I2C.

@lobradov
Copy link
Contributor Author

I see what you mean. I can do it - give me a minute.

Copy link
Contributor

@Skaronator Skaronator left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@xoseperez xoseperez merged commit 15d69d9 into xoseperez:dev Apr 11, 2018
@xoseperez
Copy link
Owner

Let's go!

@xoseperez
Copy link
Owner

xoseperez commented Apr 11, 2018

I had to change some stuff (main problem was with DEBUG_MSG definition order but got it working. Also changed the way the ADC_MODE setting is defined.

I think it's a good step. Things look more rational now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants