mem:
I think having a one second default is a better idea. It won't block unless the device doesn't respond and in that case does it matter if the error is returned in one second rather than 250ms? (in the previous code, the error condition would cause it to block indefinitely)
The advantage of a longer default time-out is that inexperienced users won't get false errors when using devices that do take longer than 250ms. Experienced users can increase or decrease the time-out if they want, but it seems more friendly to have a default that minimizes the chance that errors could be reported even when the system if functioning correctly. The longer default time-out has no impact on performance on systems where the I2C devices are responding correctly.
Your last statement here (longer timeout values have no impact on working systems) is definitely true, and it means that I don't have a huge reason not to increase the default timeout to 1 sec. But let me elaborate on my interpretation of the situation in one last effort to stomp my proverbial foot like a spoiled three-year-old so I can still get my own way: (kidding)
The I2Cdev class is the convenience layer between actual device libraries and the TwoWire I2C implementation library (or alternative implementation in the future). This is where the default timeout of 250ms is defined currently, and it's the I2Cdev's read* methods that allow for alternate timeout values as part of their arguments. The I2Cdev object itself is not intended to be used by itself in your projects (though you could if you really wanted to). Instead, it's designed to be used by device libraries like the ADXL345 class or ITG3200 class. These classes wrap the I2C communication into more intelligible methods, like "getAcceleration(x, y, z)" or "getRotation(x, y, z)". The implementations of these functions, per device and per class method, can individually set their own timeouts.
The important bit here is that all of the I2Cdev calls and which device operations require which timeouts should be entirely transparent to the person implementing any particular device class in a project, a.k.a. the "inexperienced users" who would likely make a mistake with the timeout setting. The only time anyone has to worry about specific timeout settings is if they are actually writing a new device class, in which case it seems very likely that they would know to specify an extra long timeout value in their "observeTortoiseMarathonPath()" method.
For this reason, it seems more useful overall to have a legitimate failure come back faster rather than slower. But honestly, I'm willing to change it to 1 sec if you would still recommend it in light of the viewpoint I laid out above (and maybe you clearly understood all of that before and my explanation was redundant anyway). No hard feelings in either case; I'm just trying to be diligent.
retrolefty:
Thanks Jeff for the great explanation, and of course your latest 'fix' compiles and runs fine. I'm looking forward to playing with this new device and your library over the weekend (playing with the grandkids today mostly). Not sure I will be able to validate all the features, the Comparator modes are not real clear to me...
Lefty, this page has some very good info on typical comparator features, including hysteresis and latching:
Some of it is kind of hard for me to parse through since I'm still more of a software guy than a hardware guy, but if I understand correctly, the nutshell version is that using hysteresis instead of a pure window mode helps reduce the effects of noise and prevents rapid negative/positive output "bouncing" near 0V levels, and latching mode will cause the ALERT_RDY pin to remain high until the data is read even if the actual state changes before it is read. I could be wrong about either of these points, and I welcome clarification from anyone who actually knows what they are talking about.
Don't worry about testing every bit of functionality unless you actually want to; as long as the basic I/O and core functions work for you, that's the important part, and it's a good indication that the rest of the library should work as defined (unless I made a typo, which has happened on numerous previous occasions).
Jeff