On 03-01-15 07:39, Linus Torvalds wrote:
The really sad part is that the EON Steel handles gas change events correctly, by actually saying which cylinder it switches to. But the libdivecomputer interfaces are broken, and only contain the gas *mix* you switch to, which is ambiguous since you could have the same mix in multiple cylinders.
Unfortunately I can't really fix this mistake right now, without breaking backwards compatibility. Otherwise I would have addressed this already.
Maybe we could put the one-based cylinder index into the "flags" field? With zero meaning "unknown". That would be a straightforward extension.
I really don't like this. First of all, the flags field are supposed to be used for the begin/end flags. Abusing that field for storing the gasmix index is the same kind of ugly hack that resulted in the SAMPLE_EVENT_GASCHANGE2 encoding. I would like to get rid of those, not add more! A one based index is also very confusing, especially because we already use a zero based index in the dc_tank_t structure.
How about an alternative solution? Instead of trying to re-use the existing SAMPLE_EVENT_GASCHANGE2 event, why not introduce a new SAMPLE_EVENT_GASCHANGE3 containing just the gasmix index? It maintains backwards compatibility without needing any ugly hacks. After v0.5 is released, we can deprecate the other two (but leave the constants in place) and port all backends to the new event for a v0.5.1 release shortly afterwards. That way, applications will have a clear timeframe for migrating to the new model. So that looks like a sane compromise to me.
Actually, instead of yet another gas change event, I believe we should model the active gasmix as a DC_SAMPLE_GASMIX sample type (also containing the zero based gasmix index as the value). That's similar to what we already have for setpoint changes, and also a move in the right direction for the new api design. See the attached patch.
Jef