drivers icon indicating copy to clipboard operation
drivers copied to clipboard

all: introduce a temperature type

Open aykevl opened this issue 3 years ago • 3 comments

This type should be used whenever a sensor (or actuator?) works with a temperature. For example, this commit changes the signature:

ReadTemperature() (int32, error)

to the following:

ReadTemperature() (drivers.Temperature, error)

I believe this is much clearer in intent. It also makes it trivial to introduce common conversions. For example, there are already Celsius() and Fahrenheit() methods to convert to the given units, as a floating point. More units could be added as needed, for example a CelsiusInt().


This PR has a few possibly controversial changes:

  • I've changed the DHT driver a bit to be more in line with other drivers
  • I've removed the ReadTempC and ReadTempF functions from the adt7410 driver because those are now unnecessary

I am aware that this is a breaking change. However, we use versioning and Go modules for a reason, and I think this change will improve the drivers in general. I intend to also apply a similar change to other units such as pressure (and maybe acceleration etc.)

aykevl avatar Oct 21 '21 21:10 aykevl

This seems out of line with what I feel is the purpose of the drivers namespace, to provide a hardware abstraction layer. This seems more natural-sciences or physics library oriented. It also conflicts with this PR https://github.com/tinygo-org/drivers/pull/321/files, which implements the Temperature type to abstract a sensor type.

soypat avatar Oct 22 '21 11:10 soypat

Would it be worth introducing a physics-like package, much like the periph.io package?

soypat avatar Oct 22 '21 11:10 soypat

This seems out of line with what I feel is the purpose of the drivers namespace, to provide a hardware abstraction layer. This seems more natural-sciences or physics library oriented. [...] Would it be worth introducing a physics-like package, much like the periph.io package?

The Temperature type was indeed based on the same idea of periph.io. I didn't put it in a different package as I didn't see a need for it: it seemed to me like the base drivers package is good enough for that. I like to keep things in the same package unless there is a good reason not to.

It also conflicts with this PR https://github.com/tinygo-org/drivers/pull/321/files, which implements the Temperature type to abstract a sensor type.

Right. I didn't think of that. Maybe that's a good reason to move it to a different package.

aykevl avatar Oct 22 '21 16:10 aykevl