[PATCH] Limit to 4 the number of cylinders shown in the data frame

[Dirk Hohndel]

>> On Dec 26, 2012, at 1:25 PM, Salvador Cuñat wrote:
>> 
>> > Good night.
>> > Here are attached  a pair of printouts of the test dives.  I have modified a pair of them, adding more text, weight and dummy tank data.
>> > As you can see, there is room for more data, probed they are not very big.

I think this progressing really nicely. Thanks for working on our print out routines!

A couple of comments on the code…

>> > +               while ( n < tank_count && n < first_tank + 4){
>> > +                       cairo_translate (cr, 0, height / (double) PANGO_SCALE);
>> > +                       cairo_move_to(cr, 0, 0);
>> > +                       cyl_cap = get_volume_units(dive->cylinder[n].type.size.mliter, &decimals, &unit_vol);
>> > +                       cyl_wp = get_pressure_units(dive->cylinder[n].type.workingpressure.mbar, &unit_press);
>> > +                       cyl_press= get_pressure_units(dive->cylinder[n].start.mbar, &unit_press);
>> > +                       cyl_end= get_pressure_units(dive->cylinder[n].end.mbar, &unit_press);
>> > +                       cyl_cons_gas = (cyl_press - cyl_end) * cyl_cap;
>> > +                       curwidth = 0;
>> > +                       i = 0;
>> > +                       while ( i < 3 ) {
>> > +                               cairo_move_to(cr, curwidth / (double) PANGO_SCALE, 0);
>> > +                               switch(i) {
>> > +                                       case 0 : if (prefs.output_units.volume == CUFT) {
>> > +                                               cyl_cap *= cyl_wp / 14.7 ;
>> > +                                               }
>> > +                                               snprintf(buffer, sizeof(buffer), _("%.*f %s"),
>> > +                                                       decimals,
>> > +                                                       cyl_cap,
>> > +                                                       unit_vol);
>> > +                                       break;
>> > +                                       case 1 : cairo_move_to(cr, curwidth / (double) PANGO_SCALE, 0);
>> > +                                               if (dive->cylinder[n].gasmix.o2.permille == 0){
>> > +                                                       snprintf(buffer, sizeof(buffer), _("air"));
>> > +                                               } else {
>> > +                                                       snprintf(buffer, sizeof(buffer), _("%d/%d"),
>> > +                                                       dive->cylinder[n].gasmix.o2.permille /10,
>> > +                                                       dive->cylinder[n].gasmix.he.permille /10);
>> > +                                               }
>> > +                                       break;
>> > +                                       case 2 : cairo_move_to(cr, curwidth / (double) PANGO_SCALE, 0);
>> > +                                               if (prefs.output_units.volume == CUFT) {
>> > +                                                       cyl_cons_gas /= 14.7 ;
>> > +                                               }
>> > +                                               snprintf(buffer, sizeof(buffer), _("%.*f %s\n"),
>> > +                                                       decimals,
>> > +                                                       cyl_cons_gas,
>> > +                                                       unit_vol);
>> > +                                       break;
>> > +                                       default : snprintf(buffer, sizeof(buffer),"Error");
>> > +                               }
>> > +                               pango_layout_set_text(layout, buffer, -1);
>> > +                               pango_cairo_show_layout(cr, layout);
>> > +                               curwidth += (maxwidth/ 3);
>> > +                               i++;
>> > +                       }

I'm not in love with that loop. It goes from 0 to 2 and the majority of its content is that switch statement with three special cases for the three values of i.
Is this really a loop or is this better handled by sequential code with useful helper functions or macros? I just don't think this code is really intuitive.

Speaking of helper functions… please use get_volume_units to deal with the cuft/liter display. Much cleaner and more consistent with the rest of the code.
The display of EAN32 as (32/0) is not necessarily wrong, but might be unfamiliar to most people. Wouldn't it be better to only use the trimix style naming
(O2%/He%) when He > 0 and otherwise call this EAN32 or 32%Nitrox or something?

>> > -       /*Create a frame to separate tank area, note the scaling*/
>> > -       cairo_rectangle(cr, 0, 0 + h*0.05, w, h*0.9);
>> > -       cairo_set_line_width(cr, 0.01);
>> > -       cairo_set_line_join(cr, CAIRO_LINE_JOIN_MITER);
>> > -       cairo_stroke(cr);
>> > +       /* Calculate total weight
>> > +        * copy/paste from divelist.c
>> > +        */
>> > +       double total_weight(struct dive *dive)
>> > +       {
>> > +               int i, total_grams = 0;
>> > +
>> > +               if (dive)
>> > +                       for (i=0; i< MAX_WEIGHTSYSTEMS; i++)
>> > +                               total_grams += dive->weightsystem[i].weight.grams;
>> > +               return total_grams;
>> > +       }

Why copy and paste a function? Why not just call the function in divelist.c?

>> > +
>> > +       /* Print weight system */
>> > +       void print_weight_data (int maxwidth, int maxheight, int height){
>> > +               int decimals,i;
>> > +               double totalweight, totalweightsystem, weightsystemcounter;
>> > +               const char *unit_weight;
>> > +               char buffer[80];
>> > +               PangoLayout *layout;
>> >
>> > -       maxwidth = w * PANGO_SCALE;
>> > -       maxheight = h * PANGO_SCALE * 0.9;
>> > +               layout = pango_cairo_create_layout(cr);
>> > +               pango_layout_set_height(layout,maxheight);
>> > +               pango_layout_set_width(layout, maxwidth);
>> > +               pango_layout_set_wrap(layout, PANGO_WRAP_WORD_CHAR);
>> > +               set_font(layout, font, FONT_SMALL - 3, PANGO_ALIGN_CENTER);
>> >
>> > -       /* We need to know how many cylinders we used*/
>> > -       for (z = 0; z < MAX_CYLINDERS; z++ ){
>> > -               if (dive->cylinder[z].start.mbar == 0) {
>> > -                       break;

Oops - this looks like a little too much copy and paste… aren't we dealing with weight systems here?

>> > +       /* Print the dive OTUs */

I honestly don't know how many people care about OTUs. I added that a while ago to the available information but so far found very few people who consider it useful. So I'm not sure I would devote space to this.

>> > +       /* Print the dive maxCNS */

This is more useful, especially once I find the time to finish our own CNS calculations for all the dive computers that don't report CNS information.

Can you rework those issues and submit another patch, please?

Thanks

/D