Skip to content

setResolution() and other functions looping over devices inefficient and dangerous. #193

@MalcolmBoura

Description

@MalcolmBoura

setResolution() and other functions looping by index exhibit O(n^2) duration.

They also send DS18 commands to every device on the bus, even those that are not DS18, which wastes time and may do something unexpected in consequence.

Steps To Reproduce Problem

Inspect the code.

Looping over all devices using for(i=0; i<devices; i++) { ... getAddress(); ...} is inherently inefficient because getAddress() carries out a linear search of the bus from the start for every call. This leads to O(n^2) behaviour.

Many functions are also dangerous because they apply DS18 commands to every device on the bus even those that are unsupported and may do something unexpected in consequence.

The fix is to provide an iterator, similar to that provided by OneWire, which iterates over only supported devices. It is both more efficient and cleaner code.

// start iterator over the supported devices
void DallasTemperature::iteratorReset() {    
	_wire->reset_search();
}

// continue iteration over the supported devices
bool DallasTemperature::iteratorNext(uint8_t* deviceAddress)  {
    while (_wire->search(deviceAddress)) {
        if (validAddress(deviceAddress) && validFamily(deviceAddress))
            return true;
    }
    return false;
}

The loop in setResolution() and similarly for similar loops in other functions, then becomes

        iteratorReset();
	while (iteratorNext(address)) {
		setResolution(address, bitResolution, true);
	}

NB the above has not yet been tested as I am engaged in some other more drastic changes. More on that in due course.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions