bh1745 driver and driver example

[hoanga]

Hello!

This is a driver for the bh1745 sensor that is included on the Pimoroni enviro indoor. Confirmed the light sensors works on an available enviro board

links:
https://github.com/pimoroni/enviro
https://shop.pimoroni.com/products/enviro-indoor

[deadprogram]

Hi @hoanga thanks for the PR. Please see my feedback on #870 same applies to this PR.

[hoanga]

Updated with suggestions as per #870.

[deadprogram]

Thanks for the driver. It builds correctly. gofmt, go vet and go build all give no errors. The layout agrees with the other drivers in this repo (driver file, registers.go, example, smoketest line).

These are the items I found.

Correctness

1. The threshold register names and comments are not correct (bh1745/registers.go:19-20)

REG_THRESHOLD_LSB  = 0x62 // interrupt low threshold, little-endian uint16
REG_THRESHOLD_MSB  = 0x64 // interrupt high threshold, little-endian uint16

These are two different 16-bit registers. They are not the two halves of one register. The datasheet register map (BH1745NUC) gives TH_LSB = 0x62 and TH_MSB = 0x63 for the high threshold, and TL_LSB = 0x64 and TL_MSB = 0x65 for the low threshold. Thus the names are not correct, and the high and low comments are in the wrong order. Please use REG_TH_LSB = 0x62 and REG_TL_LSB = 0x64.

2. The package comment is not attached to the package (bh1745/bh1745.go:1-9)

There is an empty line between the comment block and package bh1745. Go does not use it as a doc comment. go doc shows only the comment from registers.go. Also, the two package comments give two different datasheet URLs (mouser and rohm). Please keep one package comment in one file, with one URL.

3. Read() has a Deprecated: note in a new driver (bh1745/bh1745.go:151-156)

There is no earlier version to be compatible with. Please remove Read() and RGBCData, or keep them and remove the deprecation note.

Style and repo conventions

4. The new driver uses internal/legacy

legacy.WriteRegister does make([]uint8, len(data)+1) at each call. Thus each register write makes a heap allocation. New drivers must call d.bus.Tx(d.Address, []byte{reg, val}, nil) directly. See bh1750/bh1750.go for an example.

5. SetMeasurementTime and SetADCGain discard errors (bh1745/bh1745.go:106-118)

All the other methods return an error. These two hide I2C failures. Also, SetADCGain does a read, then a write, but ignores the read error. If the read fails, it writes a gain value that comes from an empty buffer.

6. CalcColorTemperature uses float64 and math.Exp

Software float64 and Exp are slow on Cortex-M0 and M0+ targets, and they increase the flash size. CalcLux already uses float32. Please use float32 in both functions, if the accuracy is sufficient.

7. CalcLux does not use its b parameter (bh1745/bh1745.go:161)

The function accepts b, but the formula uses only r, g and c. This is acceptable if you want the same signature as CalcColorTemperature. A short comment would make it clear.

Minor

• AddressAlt (0x39) is declared, but no constructor selects it. The user must set the exported Address field. A NewWithAddress function, or a comment, would help.
• Connected() ignores both read errors. Thus a bus failure gives the same result as a sensor that is not present. The older drivers do the same, so no change is necessary.
• Some comments give more detail than usual for this repo. For example, bh1745.go:38-42 repeats what the code below shows. But the MODE_CONTROL3 note with the datasheet reference is useful. Please keep it.

The driver works, and you confirmed it on hardware. Please correct items 1, 2 and 3 before the merge. Items 4 and 5 make the driver agree with the newer drivers.