From: Linus Torvalds torvalds@linux-foundation.org Date: Fri, 2 Jan 2015 22:23:40 -0800 Subject: [PATCH] Add EON Steel gas change event parsing
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.
Maybe we could put the one-based cylinder index into the "flags" field? With zero meaning "unknown". That would be a straightforward extension.
Signed-off-by: Linus Torvalds torvalds@linux-foundation.org ---
It's really sad how this loses the cylinder number information, but other than that it seems to work for me.
There's a lot of other events that I don't parse, and that I now have that Don took the EON Steel down to 200 ft with trimix and sharks, but this is the primary missing one.
src/suunto_eonsteel_parser.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)
diff --git a/src/suunto_eonsteel_parser.c b/src/suunto_eonsteel_parser.c index 6c4f4aba3d39..d5bfb0e74942 100644 --- a/src/suunto_eonsteel_parser.c +++ b/src/suunto_eonsteel_parser.c @@ -291,6 +291,25 @@ static void sample_cylinder_pressure(struct sample_data *info, unsigned char idx if (info->callback) info->callback(DC_SAMPLE_PRESSURE, sample, info->userdata); }
+static void sample_gas_switch_event(struct sample_data *info, unsigned short idx) +{ + suunto_eonsteel_parser_t *eon = info->eon; + dc_sample_value_t sample = {0}; + int o2, he; + + if (idx < 1 || idx > eon->cache.ngases) + return; + + // Horrible, broken, gas change events + o2 = 100 * eon->cache.gasmix[idx-1].oxygen; + he = 100 * eon->cache.gasmix[idx-1].helium; + + sample.event.type = SAMPLE_EVENT_GASCHANGE2; + sample.event.value = o2 | (he << 16); + + if (info->callback) info->callback(DC_SAMPLE_EVENT, sample, info->userdata); +} + static int traverse_samples(unsigned short type, const struct type_desc *desc, const unsigned char *data, int len, void *user) { struct sample_data *info = (struct sample_data *) user; @@ -311,6 +330,9 @@ static int traverse_samples(unsigned short type, const struct type_desc *desc, c case 0x000a: // cylinder idx in first byte, pressure in next word sample_cylinder_pressure(info, data[0], array_uint16_le(data+1)); break; + case 0x001d: + sample_gas_switch_event(info, array_uint16_le(data)); + break; } return 0; }
All three patches in this series are now in the Subsurface-testing branch...
I know that Jef is working on cylinder based switching - I also know that he is quite busy with his real life right now, so no idea how long it will take to get this implemented.
I'd be ok with passing this as a flag to Subsurface. one-based is kind of awkward but seems to make the most sense as 0 really should mean "unknown" in this context.
Feel free to send patches that implement that on both sides :-)
/D
On Fri, Jan 02, 2015 at 10:39:21PM -0800, Linus Torvalds wrote:
From: Linus Torvalds torvalds@linux-foundation.org Date: Fri, 2 Jan 2015 22:23:40 -0800 Subject: [PATCH] Add EON Steel gas change event parsing
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.
Maybe we could put the one-based cylinder index into the "flags" field? With zero meaning "unknown". That would be a straightforward extension.
Signed-off-by: Linus Torvalds torvalds@linux-foundation.org
It's really sad how this loses the cylinder number information, but other than that it seems to work for me.
There's a lot of other events that I don't parse, and that I now have that Don took the EON Steel down to 200 ft with trimix and sharks, but this is the primary missing one.
src/suunto_eonsteel_parser.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)
diff --git a/src/suunto_eonsteel_parser.c b/src/suunto_eonsteel_parser.c index 6c4f4aba3d39..d5bfb0e74942 100644 --- a/src/suunto_eonsteel_parser.c +++ b/src/suunto_eonsteel_parser.c @@ -291,6 +291,25 @@ static void sample_cylinder_pressure(struct sample_data *info, unsigned char idx if (info->callback) info->callback(DC_SAMPLE_PRESSURE, sample, info->userdata); }
+static void sample_gas_switch_event(struct sample_data *info, unsigned short idx) +{
- suunto_eonsteel_parser_t *eon = info->eon;
- dc_sample_value_t sample = {0};
- int o2, he;
- if (idx < 1 || idx > eon->cache.ngases)
return;- // Horrible, broken, gas change events
- o2 = 100 * eon->cache.gasmix[idx-1].oxygen;
- he = 100 * eon->cache.gasmix[idx-1].helium;
- sample.event.type = SAMPLE_EVENT_GASCHANGE2;
- sample.event.value = o2 | (he << 16);
- if (info->callback) info->callback(DC_SAMPLE_EVENT, sample, info->userdata);
+}
static int traverse_samples(unsigned short type, const struct type_desc *desc, const unsigned char *data, int len, void *user) { struct sample_data *info = (struct sample_data *) user; @@ -311,6 +330,9 @@ static int traverse_samples(unsigned short type, const struct type_desc *desc, c case 0x000a: // cylinder idx in first byte, pressure in next word sample_cylinder_pressure(info, data[0], array_uint16_le(data+1)); break;
- case 0x001d:
sample_gas_switch_event(info, array_uint16_le(data));break;}
2.2.1.212.gc5b9256
On Sat, Jan 3, 2015 at 3:51 PM, Dirk Hohndel dirk@hohndel.org wrote:
Feel free to send patches that implement that on both sides :-)
Done. See the commentary about possible issues (mostly "we don't have a lot of testing around cylinder indexes in gas switch events")
I'm probably done for the day - have to drive kids around etc.
Linus
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
-
Dirk Hohndel -
Jef Driesen -
Linus Torvalds