TheThingsIndustries / generic-node-se

Generic Node Sensor Edition
https://www.genericnode.com
Other
110 stars 31 forks source link

Feature/basic sensor serial #34

Closed elsalahy closed 4 years ago

elsalahy commented 4 years ago

Summary:

Adds support for SHTC3 sensor driver and the Sensors serial I2c Bus.

Relates to #30

Changes:

Notes for Reviewers:

None

Release Notes: (optional)

None

elsalahy commented 4 years ago

@johanstokking I tried to avoid doing changes to the SHTC3 driver, and took at as it is with some minor header fixes and I2C implementation.

But I was wondereing if I should fix the commenting style in these files to make it match our style, we did this in Mbed, but not sure with baremettal if it's worth it. What do you think?

johanstokking commented 4 years ago

But I was wondereing if I should fix the commenting style in these files to make it match our style, we did this in Mbed, but not sure with baremettal if it's worth it.

I'm undecided on this. On one hand it's useful to avoid diffs with dependencies that are pasted in, on the other hand it would make the tooling and formatting easier. My feeling says not doing it.

elsalahy commented 4 years ago

But I was wondereing if I should fix the commenting style in these files to make it match our style, we did this in Mbed, but not sure with baremettal if it's worth it.

I'm undecided on this. On one hand it's useful to avoid diffs with dependencies that are pasted in, on the other hand it would make the tooling and formatting easier. My feeling says not doing it.

I feel the same, I think for now, l would leave it as it is and In the meantime, I opened this #38 to keep track of things.

elsalahy commented 4 years ago

@azerimaker and @johanstokking did you get a a chance to review?

johanstokking commented 4 years ago

Go ahead merging