[PATCH] For CCR dives show O2 setpoint graph.

Willem Ferguson willemferguson at zoology.up.ac.za
Sat Jan 3 12:24:24 PST 2015



Thanks for the comments.

On 03/01/2015 20:24, Dirk Hohndel wrote:
> On Sat, Jan 03, 2015 at 08:32AM -0800, Willem Ferguson wrote:
>> Subject: [PATCH] Show the o2 setpoint graph
>>
>> When a CCR dive is viewed and the toolbar button for PO2 is activated,
>> both the PO2 (green line) and the O2 setpoint (red line) are shown.
>> This allows evaluation of the PO2 in the CCR loop with respect to the
>> pre-configured O2 setpoint.
>>
>> I need someone who is conversant with Qt to check this code, Please!
> Tomaz is traveling, I'm sure he'll look at it when he's back (I didn't
> approve his 2MB email showing his massive new tattoo where he said that).
>
> But here are a few comments from me...
>
>> diff --git a/profile.c b/profile.c
>> index 74d4936..54ee84e 100644
>> --- a/profile.c
>> +++ b/profile.c
>> @@ -608,7 +607,7 @@ struct plot_data *populate_plot_entries(struct dive *dive, struct divecomputer *
>>   		entry->in_deco = sample->in_deco;
>>   		entry->cns = sample->cns;
>>   		if (dc->dctype == CCR) {
>> -			entry->o2pressure.mbar = sample->setpoint.mbar;     // for rebreathers
>> +			entry->o2pressure.mbar = entry->o2setpoint.mbar = sample->setpoint.mbar;     // for rebreathers
> So why do we need a second field in the entry to hold this information?
The o2pressure field is ultimately used to store the measured po2. But
it is initialised with the
setpoint data because setpoint is the best initial guess of what the po2
is likely to be. BUT: this is a
remnant of code from before Robert created the gas_pressures structure.
In previous correspondence
some months ago we agreed that this needs to changed for consistency: we
have not done
that yet. This duplication should be cleaned up, but probably not as
part of this patch. If you would
trust me with this, I could do this cleanup within the next week.
>
>> diff --git a/qt-ui/graphicsview-common.cpp b/qt-ui/graphicsview-common.cpp
>> index 4beab9d..982cb84 100644
>> --- a/qt-ui/graphicsview-common.cpp
>> +++ b/qt-ui/graphicsview-common.cpp
>> @@ -26,7 +26,8 @@ void fill_profile_color()
>>   	profile_color[PN2] = COLOR(BLACK1_LOW_TRANS, BLACK1_LOW_TRANS, BLACK1_LOW_TRANS);
>>   	profile_color[PN2_ALERT] = COLOR(RED1, BLACK1_LOW_TRANS, RED1);
>>   	profile_color[PHE] = COLOR(PEANUT, BLACK1_LOW_TRANS, PEANUT);
>> -	profile_color[PHE_ALERT] = COLOR(RED1, BLACK1_LOW_TRANS, RED1);
>> +	profile_color[PHE] = COLOR(PEANUT, BLACK1_LOW_TRANS, PEANUT);
>> +	profile_color[O2SETPOINT] = COLOR(RED1, BLACK1_LOW_TRANS, RED1);
> So this delete's the PHE_ALERT color and duplicates the PHE color.
> Why is that a good thing?
This is a glitch on my side. The top + should be a - and the present -
should be a +.
Only the last line is an addition to the present code. The rest is noise
due my fiiddling
with the code.

>
>> diff --git a/qt-ui/profile/diveplotdatamodel.cpp b/qt-ui/profile/diveplotdatamodel.cpp
>> index 51aea1b..aff9ebe 100644
>> --- a/qt-ui/profile/diveplotdatamodel.cpp
>> +++ b/qt-ui/profile/diveplotdatamodel.cpp
>> @@ -52,6 +52,11 @@ QVariant DivePlotDataModel::data(const QModelIndex &index, int role) const
>>   			return item.pressures.he;
>>   		case PO2:
>>   			return item.pressures.o2;
>> +		case O2SETPOINT:
>> +			if (item.o2setpoint.mbar > 5000)
>> +				return(0);
> What is special about setpoints larger than 5bar (except that it might
> kill you, I mean)? How could we ever get such a setpoint? And if we did,
> why would we want to hide that by returning 0?
The Poseidon sometimes returns a crazy value for setpoint at the very
start of the dive.
This is just a filter to prevent off-scale values. Maybe one should do
this filtering during
the transfer from structures of data to structures of plot_info??
Ideally at import stage
but, if CCR setpoint data are imported from various sources, this is
maybe more difficult.

>
>> +				return item.o2setpoint.mbar / 1000.0;
> Why are you returning bar? The other entries here appear to return a
> pressure_t
The gas_pressures structure has pn2, po2 and phe of format double, not
pressure_t.
So there is no .mbar member in gas_pressures since the unit there is in
bar (see the
function fill-pressures() in dive.c).  If I present mbar for the
setpoint graph, the setpoint is
way off-scale on the profile. Within the context of this argument there
may be grounds for
reconsidering some of the units issues while dealing with the o2pressure
variable, discussed
above. But every single thing that Robert does is so well thought-through
that I may be revealing my ignorance on these issues.

>
>> diff --git a/qt-ui/profile/profilewidget2.cpp b/qt-ui/profile/profilewidget2.cpp
>> index d631e3c..c1bc402 100644
>> --- a/qt-ui/profile/profilewidget2.cpp
>> +++ b/qt-ui/profile/profilewidget2.cpp
>> @@ -521,6 +525,11 @@ void ProfileWidget2::plotDive(struct dive *d, bool force)
>>   		currentdc = fake_dc(currentdc);
>>   	}
>>
>> +	if (currentdc->dctype == CCR) {
>> +		o2SetpointGasItem->setVisible(true);
>> +	} else {
>> +		o2SetpointGasItem->setVisible(false);
>> +	}
> So the user doesn't get to turn this off if it's a CCR dive? Or is this
> somewhere else turned of if the user doesn't want to see partial
> pressures? I only see it turned off below for the empty state...

My personal opinion is that, when viewing the po2 graph, the most
important single consideration of a CCR diver
is the degree to which the measured po2 reflects the setpoints
(remember, setpoints can potentially be
changed during the dive). For that reason I hard-wired it. BUT: this is
a personal bias and an additional option
in the preferences panel is an immanent possibility.
Kind regards,
willem





More information about the subsurface mailing list