adafruit / Adafruit_CircuitPython_SI1145

CircuitPython helper library for the SI1145 Digital UV Index IR Visible Light Sensor
MIT License
0 stars 5 forks source link

Add UV Index, use adafruit_register #4

Closed tekktrik closed 2 years ago

tekktrik commented 2 years ago

I don't have the hardware but did by best to resolve #1 by adding the UV index functionaliy. Some other changes happened along the way:

caternuson commented 2 years ago

Is using register necessary? It's a simple sensor and bringing in register will make it potentially not usable on small boards.

tekktrik commented 2 years ago

Probably not necessary, I really just noticed that some of the functionality here could be covered by using it. I think that was just my understanding of the design guide, but definitely understand the argument for avoiding it. This shouldn't be too difficult to refactor out if we go that route.

caternuson commented 2 years ago

Added @adafruit/circuitpythonlibrarians as reviewers so others can comments.

tekktrik commented 2 years ago

Oh never mind, the design guide only specifies adafruit_bus_device, whoops! I can refactor it out if you think that's the best move.

caternuson commented 2 years ago

My two cents is to leave it out for now. I only consider using the register lib for a device if there are a lot of registers broken up into various smaller bits that control a guzillion internal settings. The SI1145 only has a few, like IRQ_ENABLE and IRQ_STATUS.

But the counter argument is the abstracting out of all the bit shifting / masking foo that the register lib provides for the sake of code safety.

tekktrik commented 2 years ago

I'm out during the next meeting, but maybe this worth bringing up on the Monday meeting? I can add it and just listen to the meeting after the fact.