On Tue, Oct 28, 2014 at 02:58:50PM -0700, Linus Torvalds wrote: > On Tue, Oct 28, 2014 at 2:13 PM, Dirk Hohndel <d...@hohndel.org> wrote: > > > > Bug #742 shows that we got things pretty badly wrong for the Cobalt if the > > primary gas used on a dive (or in the case of his sample data, the only > > gas used on a dive) was not tank 0. We showed that ugly gas change, we > > showed that two gases were used during the dive, we associated the > > pressure from the samples with the wrong tank, etc. > > > > I just pushed out a commit that pretends to fix this. And it makes sense > > to me. But I'd really appreciate if you (or anyone else for that matter) > > would take a look and double check that this is ok. > > It looks fine, but the whole "<= 30s" and "first sample" thing looks a > bit confused. Do you really need *both*?
From reading the Cobalt code we don't need the <= 30s code anymore - it's ALWAYS on the first sample. I wondered if there was another dive computer that did something similar where we might still need it. I can't think of one, though. Maybe I should just remove it and see if something breaks :-) > Also, that whole > > /* if we have an explicit first cylinder */ > if (sample->sensor == 0 && first_cylinder != 0) > sample->sensor = first_cylinder; > > looks a bit iffy. I guess we're stuck with it, but my gut feel is that > it's actually an Atomic Cobalt back-end bug, and that the Cobalt > should just have reported the right sensor, rather than reporting > sensor 0. I was wondering about that as well. The backend clearly has code there to try to report this data - but without access to a Cobalt this is hard to figure out... the Cobalt is another one of the dive computers where we can't just do a memory dump. Jef - I think you were working on a tool to dump the raw data for a dive? That might be helpful here... Or I could just send the user a hacked version of Subsurface that dumps the raw data to a file. He is responsive on trac... > The reason it's iffy is that another dive computer that does this > *right* might be impacted. And that's exactly why I wanted to show this to you... > But right now, I guess only the Cobalt has > this whole "first_cylinder" issue, so maybe worry about that > possibility later.. And quite frankly, we'll need to eventually fix > the fact that we can only have one cylinder pressure per sample, so > this code will end up needing changes in more fundamental ways > eventually. So the whole "this might break in the future" is not a > very strong objection. The code is pretty much *guaranteed* to break > in the future for other reasons anyway. Lovely. See my comment above :-/ Thanks for taking a look. I'll remove the <= 30s checks and leave it as is otherwise. /D _______________________________________________ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface