On Sat, Aug 24, 2024 at 01:29:24PM +0200, Vasileios Amoiridis wrote: > On Fri, Aug 23, 2024 at 10:25:01PM +0300, Andy Shevchenko wrote: > > On Fri, Aug 23, 2024 at 08:17:11PM +0200, Vasileios Amoiridis wrote: ... > > > + meas_time = 4000 + time_conv_temp[data->oversampling_temp] + > > > + time_conv_press[data->oversampling_press]; > > > > 4 * USEC_PER_MSEC ? > > Since the previous values in the arrays are all in thousands, why should > I make this different? When I read the code (and mind that we write code for humans), I don't have a clue about the order of the values in use. Also it's hard to get from the line the meaning of both sides of the formula. Using named definitions helps a lot in understanding this line without reading and analysing code in full. ... > > > + usleep_range(meas_time, meas_time * 12 / 10); > > > > Comment? fsleep() ? > > The usleep here is for waiting for the sensor to make the conversion, > as the function name points out as well? Should I put it as a comment? > > In general, is it considered good practice to add comments above all > sleep functions? Yes, it's even a requirement (not sure if it's documented anywhere) to comment over long enough delays. > I don't think it's a bad idea, I just didn't notice > it somewhere. > > > > + return 0; > > > +} ... > > > + usleep_range(2500, 3000); > > > > fsleep() ? > > > > ACK. Also a comment, since it's milliseconds range which might be considered long enough. -- With Best Regards, Andy Shevchenko
