Perform search for setResolution linearly?

[Andersama]

As mentioned in #193 the issue stems from repeated usage of getAddress(). It doesn't seem to me to be necessary to use iterators to sort out the performance issue in the library, just a general avoidance of getAddress() outside index based functions.

Since getAddress() appears to be used to do a bit more than iterate through addresses I added validAddress() as an additional condition, it might not be needed.

I only found two locations where this appears to happen. both are in setResolution functions. So #193 may be resolved entirely by this. General advice to users of the library may be to avoid index based functions inside loops. This might be where an "iterator" interface may be useful inside the class, such that performing functions across devices remains somewhat linear.

[RobTillaart]

@milesburton
Can you approve the build ?

[RobTillaart]

looks good to me

[milesburton]

Thanks all, merged

[Andersama]

No problem, I am using the library after all. Since the original issue mentioned it I gave iterators a thought because search() would function like ++, but since any call to search() or reset_search() messes with following calls I didn't bother.

I didn't find that other one wire library that was mentioned, if you know where or what that was I'd be happy to take a look at it to see if it does anything different. The current one seems fairly small so I don't get the impression there's much that could be different.

My adhoc solution for ease of use for making everything else linear was this template function.

	template<typename Callback>
	bool forEachValidAddress(Callback callback) {
		bool last_result = false;
		DeviceAddress addr;
		
		_wire->reset_search();
		while(_wire->search(addr)) {
			if (validAddress(addr) && !(last_result = callback((uint8_t*)addr))) {
				break;
			}
		}
		
		return last_result;
	}

So that I could things like this:

bool process_address(uint8_t* addr) {
    float temp = sensors.getTempF(addr);

    if (temp < min_temp)
      min_temp = temp;
    if (temp > max_temp)
      max_temp = temp;
    
    device_count++;
    return true;
}
  
sensors.forEachValidAddress(process_address);

[RobTillaart]

Nice construct.
Question, if the call back fails - for whatever reason - the while loop is ended.
I assume this can be prematurely as not all are processed?

Should break not be continue?

A variation on your template, returns true only if all valid addresses are processed correctly

  template<typename Callback>
  bool forEachValidAddress(Callback callback) {
    bool last_result = true;     //  changed
    DeviceAddress addr;
		
    _wire->reset_search();
    while(_wire->search(addr)) {
      if (validAddress(addr) && !(last_result &&= callback((uint8_t*)addr))) {   //  changed
        continue;   // changed
      }
    }
  return last_result;   
  }

---

https://github.com/pstolarz/OneWireNg this one?

[Andersama]

Well, it's a boolean so the logic can be flipped eaither way, I was using false to exit the loop prematurely if needed. Say for example we were writing a loop to check if any temperature probe met some threshold, past the point you've seen a temperature probe hit the threshold you might not need to check any of the remaining devices.

Since we ignore invalidAddresses that case would always be continue. The edit you made with your continue actually doesn't even need the continue since it's about to jump to the top of the loop anyway, at which point the callback doesn't need to have a return value. Which actually could be a way you specialize the template, you could for example have explicitly added these to the header:

bool forEachValidAddress(bool(*callback)(uint8_t*)) {
    bool last_result = true;
    DeviceAddress addr;
		
    _wire->reset_search();
    while(_wire->search(addr)) {
      if (validAddress(addr) && !(last_result &&= callback((uint8_t*)addr))) {
        break;
      }
    }
  return last_result;  
}
bool forEachValidAddress(void(*callback)(uint8_t*)) {
    bool last_result = true;
    DeviceAddress addr;
		
    _wire->reset_search();
    while(_wire->search(addr)) {
      if (validAddress(addr)) {
        callback((uint8_t*)addr)
      }
    }
  return last_result;  
}