mp-se / kegmon

DIY scale for beer keg monitoring
MIT License
10 stars 4 forks source link

move code around and split sensors #24

Closed profawk closed 1 year ago

profawk commented 1 year ago

this splits up the temp sensor code and splits it up so it's easier to add more sensors (e.g. BME280)

mp-se commented 1 year ago

Thanks, I will take a look at this when I'm back from vacation.

mp-se commented 1 year ago

Hi!

Thanks for your contribution but before I can integrate it I would like to have the following changes:

profawk commented 1 year ago

regarding 4, I just use the struct to carry more then 1 value. I'm not really sure I understand what you mean regarding the rest, I changed a lot of usages to unique_ptr to increase the stability and readability of the code

profawk commented 1 year ago

also, I am lacking the hardware to test this. I don't have a DHT or a DS18B20. I just wanted to help out if possible :) So I would love if you could make sure this works as expected!

mp-se commented 1 year ago

also, I am lacking the hardware to test this. I don't have a DHT or a DS18B20. I just wanted to help out if possible :) So I would love if you could make sure this works as expected!

No problems, i can test it later.

On point 4,

in the temphumidity class data is stored as two floats, this could be replaced with your struct instead. Not a big deal i can fix that when i merge and test it.

profawk commented 1 year ago

understood, fixed :)

mp-se commented 1 year ago

I've now merged your code into the dev branch, thanks for the PR.

mp-se commented 1 year ago

@profawk just noticed that the codes does not compile on esp32 target, looks like the "make_unique" is not implemented on that compiler version. `

I guess the best option is to refactor the code so it can be complied. ?

profawk commented 1 year ago

this git diff should solve it, c++11_diff.txt use git apply c++11_diff.txt

profawk commented 1 year ago

btw, the compiler supports up to c++23 but arduino framework limits it to c++11 for some reason

mp-se commented 1 year ago

Strange indeed, that the newer version (esp32) does not support it but the older one does. Well its not a big deal to adjust for it. But it was a good learning experience, i have not been working as a developer for the last 10 years so I'm not really up to date on all the new additions to the language.

profawk commented 1 year ago

I want to add support for BME280. is it ok if I bring back the unique_ptr? I will make it work for esp32

mp-se commented 1 year ago

Yes, If you can make it work, you can base it on the dev branch