search

milesburton / Arduino-Temperature-Control-Library

Arduino Temperature Library
https://www.milesburton.com/w/index.php/Dallas_Temperature_Control_Library
974 stars 486 forks source link

DallasTemperature implements functions which belong elsewhere #195

Closed MalcolmBoura closed 2 years ago

MalcolmBoura commented 3 years ago

Several functions operate on the entire OneWire bus or apply to all OneWire devices. They should be part of either OneWire or a OneWire extension class. getAddress() readPowerSupply() getDeviceCount() Addresses would also benefit from being a class.

Removing the functions from DallasTemperature is likely to break things so unfortunately they have to be retained but should be deprecated and should be rewritten so that they call the replacement OneWire functions. As an intermediate step I am writing a OneWire extension class.

class OneWireExtra : public OneWire {
...
}

It should be possible to write an Address class that can be dropped in and used interchangeably with the uint8_t* approach. I have made a start on that and it looks promising.

RobTillaart commented 3 years ago

Why not propose these changes directly to maintainer of OneWire class? (pjrc.com)

MalcolmBoura commented 3 years ago

Why not propose these changes directly to maintainer of OneWire class? (pjrc.com)

I will be doing but first I would like to see the discussion of getAddress, see #166, progress further. I have some ideas on that and will make the suggestions there.

RobTillaart commented 3 years ago

As I have too much work at the moment it is hard for me to participate , sorry

MalcolmBoura commented 3 years ago

Following the useful discussion on #166 I have come to the conclusion that these methods, at least in their present form, do not really belong in DallasTemperature or OneWire.

  1. DallasTemperature because it only handles the DS18 series of devices and near relatives.
  2. OneWire because it only handles devices on a single pin. Accordingly I have started work on a OneWireManager that will keep track of device index, device address and which OneWire they are connected to. It will also make provision for initialising and storing that information.

OneWireManager::getAddress(deviceIndex); OneWireManager::getOneWire(deviceIndex) OneWire::readPowerSupply(deviceAddress) OneWire::readPowerSupply() // for the bus as a whole which is usually what is required OneWireManager::readPowerSupply(deviceIndex)

Deprecate getDeviceCount(), getDS18Count() and replace with an iterator

I have started coding and it is all falling into place quite nicely which is a good sign. When it has progressed a bit further I will publish it via github.

RobTillaart commented 3 years ago

Good action, (I try to keep reading your posts)

Please keep in mind that for the Arduino platform a lot of people are at beginner level.
Good to make a sophisticated level, but do not forget those who can just walk and are not able to run yet :)