deco code rewrite

[Jef Driesen]

On 07-12-12 17:51, Dirk Hohndel wrote:
> Jef Driesen <jefdriesen at telenet.be> writes:
>> I propose to add two new sample types then: DC_SAMPLE_SETPOINT and
>> DC_SAMPLE_PPO2. For the corresponding data types, the dc_sample_value_t
>> union is extended with two additional double's (named setpoint and
>> ppo2), which will contain the O2 partial pressure in bar.
>
> Yes - that should work and is trivial for us to support.

Looks good to me too.

>>>>> - Battery voltages (nice to have)
>>>>
>>>> I've always wondered why anyone would need this? I mean as long as
>>>> you have
>>>> enough, everything is fine, and if you don't then probably no dive
>>>> will be recorded.
>>>
>>> It's nice to download your dives and get the nice warning that your
>>> battery is low in the dive log program. Believe it or not, that's
>>> something that I intended to implement for Uemis (especially for the
>>> tank sensor battery) as it's so easy to do and so helpful to have.
>>
>> Of course it's important to know the battery status. However I see this
>> as something that's important while operating the device, and not
>> something that goes into your logbook. I mean it's very important that
>> the device shows you the battery status, so you can decide whether to
>> charge it or not. But what's the added value of having this info for
>> each dive? My point is that I consider the battery status more a
>> property of the device.
>>
>> Showing some kind of low battery warning during (or after) the download
>> is something different then what Jan meant with adding support for
>> battery voltages. I think (and correct me if I'm wrong), that Jan
>> referred to supporting the battery status in the header or even the full
>> battery profile in the samples (the predator has this).
>
> I think what would be nice is to have a way for libdivecomputer to
> report to the application "at the end of the dive the dive computer
> reported that its batteries / the tank sensor batteries are low and need
> to be charged or replaced". So this wouldn't be sample information (good
> for Shearwater to support that) but simply dive level header
> information.
>
> Just like we report salinity, etc. The tough part is to get the
> abstraction level right. My gut feeling would be to allow for a few
> discrete values and a few batteries. One option would be this:
>
> DC_FIELD_BATTERY_STATUS
>
> with value being a bitfield defined like this
>
> struct dc_battery_status {
>         battery0 : 4;
>         battery1 : 4;
>         battery2 : 4;
>         battery3 : 4;
>         battery4 : 4;
>         battery5 : 4;
>         battery6 : 4;
>         valid_fields: 3;
> };
>
> The number in valid fields tells you the first how many of them are
> indeed valid (so valid_fields = 0 means no battery info at all, valid
> fields = 7 means the maximum of 7 battery status fields are set). And
> the four bits per battery give us roughly 7% granularity. From looking
> around that seems to be good enough - Uemis gives you a 16bit value for
> the main battery level - but that's really not all that useful.
>
> The application can then decide what to do with those data. Have them
> displayed right after the download only. Or have them a permanent part
> of the divelog (in case you want to check after how many dives the tank
> sensor battery drops by one notch...).
>
> That seems clean, reasonably generic, quite flexible and completely in
> line with the current design.

Using bitfields is definitely not an option. Bitfields are just too compiler 
dependent (e.g. the order is implementation defined) and will very likely cause 
trouble when mixing msvc/mingw and in the .NET languages bindings. There is no 
reason to "waste" a few more bytes and use plain integers!

Anyway, I'm still not really convinced the battery status should be a property 
of the dive (e.g. a DC_FIELD_BATTERY thing). I don't have a concrete 
alternative, but I was more thinking in the direction of some event you receive 
during (or after) the download. Something similar to how you get the 
DC_EVENT_DEVINFO. The underlying idea is that, as you say getting some low 
battery warning is an interesting feature, but once you got such warning do you 
really need to have this info persist somewhere?. I also don't see any reason to 
have it available in every dive. If the battery is low on the most recent dive, 
do you still care about the battery status of the older dives?

At this point, I would say it's not such a critical feature (for libdivecomputer 
purpose of course), and therefore there is no need to rush and try to get this 
in. So let's concentrate on the more important features first. Once we have the 
new design implemented we can have a second look. Adding a new feature is a lot 
easier then having to fix or change something afterwards.

>>>>> - CNS % (quite nice, recorded by Computer??)
>>>>
>>>> This is already on our list.
>>>
>>> Yep. I hope to get to this for the Uemis in the next week or two and
>>> then create the CNS calculation in Subsurface. Not sure how hard this
>>> would be to sneak in as extension for libdivecomputer 0.3...
>>
>> Add another DC_SAMPLE_CNS type, with a double containing a percentage?
>
> Yes, please. We could also do an int that contains the permille (more
> resolution would definitely be fake). But in the current API design a
> double would be equally consistent. Either is fine with me.

Let's keep a double for now, to keep consistency.

Jef