search

adafruit / Adafruit_BME280_Library

Arduino Library for BME280 sensors
Other
328 stars 301 forks source link

Code cleanups #63

Closed dirkmueller closed 3 years ago

dirkmueller commented 4 years ago

A set of refactorings (no behavior change) accumulated over time.

ladyada commented 4 years ago

hiya - thanks for the PR! for this one, why did you remove the struct?

dirkmueller commented 4 years ago

@ladyada I tried to explain that in the commit message. with that extra struct, we're adding one redirect to every access (which adds code) and we add alignment padding (which adds memory footprint). this PR reduces memory consumption by 15%.

ladyada commented 4 years ago

hiya, we love fixes! refactors tend to add unintended side effects - so this code will take a while before we can test and merge, if ever - the current code works well, and we end up having to revert a large number of PRs due to unintended side effects.

if you'd like to work on improving the libraries, we recommend opening an issue - we can tell you exactly what would be best for the library in its current state, and work on small PRs.

thanx :)

dirkmueller commented 4 years ago

@ladyada thats okay, we ended up forking the library to be able to fix the problems with lower turnaround time. The code has been tested and is used in production already. if you look for fixes, there is #56 and #57 waiting for review .. I have more, but I like to do small, easy to understand PRs.

caternuson commented 3 years ago

Closing. Looks like this was resolved elsewhere in a fork?