Add optional SCD41 CO2 sensor to Enviro Indoor

[MichaelBell]

The Enviro Indoor product page suggests you might also want to buy an SCD41 carbon dioxide sensor to go with it.

This change detects whether you have an SCD41 plugged in to your Enviro Indoor, and if so reports a CO2 reading in addition to the other readings. It also takes the temperature reading from the SCD41, as it seemed to me that the one on the Indoor was reading a bit high - happy to drop that part if you prefer!

[ZodiusInfuser]

Hey Mike! You're a ⭐ ! I'll give this a try next week.

Looking at the code, perhaps it would be better to have this elevated outside of indoor.py 🤔 . The original intended place for user added sensors is in main.py. See this issue where I posted an example: #116

About the temperature reading, that's a good idea to include, though really I should get around to adding the ability to select which pieces of data get uploaded for all of Enviro (as was suggested in an issue I can no longer find...).

[MichaelBell]

Hmm, yes adding it in a more general place would probably make more sense. But as this is a "well known" sensor I feel it should be inside the Enviro framework, rather than handled like a user customization.

I suggest that on init enviro could determine what known sensors are plugged in from the I2C scan, and then in get_sensor_readings it could additionally read those sensors. I'll give that a go tomorrow.

And I think the temperature override should be a user customization - the default should just be to report everything. Looking at another open PR I guess my enviro temperature is reading high as I'm plugged in to USB.

[MichaelBell]

I've updated the PR to make this sensor board independent, and added some framework for additional sensors generally. Let me know what you think!

[ZodiusInfuser]

Thanks for making those changes Mike. That's quite a nice solution, and seems pretty easy for others to extend upon in the future.

May I ask what the yield scd41 line done? I'm not familiar with that keyword in this context

There should probably be some handling for if an i2c device at 98 if found but is not an SCD41. I suspect that would manifest as the init() or start() raising an exception.

Minor thing, but the 98 could do with being in hex, and saved in either enviro/constants.py or perhaps a new enviro/sensors/addresses.py 🤔

[MichaelBell]

The yield makes the function a generator, my idea was that if multiple sensors were detected by that function in future, then it could yield multiple modules back. Each returned module is iterated as the function yields it back to the caller at line 314.

I'll see if I can work out how things fail if an scd41 is not found and catch that safely.

For the 98, in my defence I was just matching the style at lines 13 and 15, but I agree it would be better to make those all hex and name the constants - I'll stick them in constants.py for now.

[MichaelBell]

I've made the changes described above.

I haven't tested having a different device connected at the same address as the SCD41 uses, but looking at breakout_scd41.cpp start() should throw a RuntimeError if it fails, so that's what I'm handling.

[Gadgetoid]

@MichaelBell don't fancy rebasing this when you've got a moment, please?

Would also be handy to compare notes with @sjefferson99's effort at #188

[MichaelBell]

Could you give me a poke in a couple of weeks? Then I'll be in the same location as the Enviro board to test changes.

From a quick skim, it looks like we should take the best bits from #188 changes and these - #188 has better documentation and configuration, whereas this has autodetection.

[Gadgetoid]

Could you give me a poke in a couple of weeks?

I'll try 😆

No hurry, I'm just trying to figure out what shape an Enviro 1.0.0 release might take.

[sjefferson99]

@MichaelBell I will take a look and familiarise myself with my own code again and yours and also try for the reminder for you. Be nice to see some variant of these PRs in v1.

[MichaelBell]

I've done the easy bit and rebased, and my CO2 sensor is still working, so that's good :)

On the other PR - I'll see if I can make a combined version.

[MichaelBell]

@Gadgetoid OK now I've raised a new PR for the combined change, let me know what you think - but hopefully we can close this and #188 and just merge that one.