On Wed, Mar 23, 2016 at 08:47:36AM +0100, Robert Helling wrote: > > you caught me here but your „solution“ actually makes it worse:
Ha. > The problem is when switching from PSI to bar: You start, say with a value of > 580psi (roughly 40bar). Then you change units, this makes the above code > execute. Now you reduce the maximum to 100 which changes the value displayed > from 580 to 100. This in turn sends a value changed signal. The receiving > slots checks units and finds „bar“, and so interprets the 100 as 100bar and > sets the prefs.reserve_gas to 100000. And only then the above code is further > executed but now thinks 100bar is what the user wants. Oops. I tested this only in the other direction. And should of course have tested both directions. > When I wrote this yesterday, I had (somewhat) reasonable values for the > maximum first (after I encountered that there is a maximum, probably coming > from the .ui file which is way to low for a psi value). The I ran into the > issue I described above. But as you noted, this was around midnight and I was > not able to solve this properly. So I just thought „what the heck“ and made > the maximum values so big that they did not interfere anymore with reasonably > sized reserves. This obviously was a hack of somebody who desperately wanted > to go to bed and you caught me. So I did half of my maintainer job :-) > What I should have done instead would have been either > > a) Move the setMaximim() call after the setValue() call in the metric branch > which avoids the above issue (my thinking was not clear enough to realize > this would have been the appropriate thing) That sounds like a good idea. > > or > > b) Get rid of the maximum value properly (not by making it ridiculously big > but by also deleting it from the .ui file). In any case, what is it good for? > Why do we want to prevent the user from doing stupid things? Because we could > get crazy values when turning the mouse wheel? And what is an appropriate > maximum? 100bar since no sane person would keep a reserve that big? Or 300bar > since this is roughly the highest pressure we would ever encounter no matter > if it makes sense as a reserve? I think having a maximum there is reasonable. We could make it 300bar/5000psi and be done with it. Who knows, maybe there is GUI++ that says "never use more than half your tank because an Asteroid could hit the earth while you are diving". > Please let me know your preference and I will send a new patch. Do you want > one with —amend or a second one on top of your modification? Since this was already pushed out, it has to be a second patch that fixes things. Otherwise this would change a git repository that others have pulled from and pulls wouldn't be 'fast forward' anymore. /D _______________________________________________ subsurface mailing list [email protected] http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
