On Mon, 18 Jun 2007 08:52:12 -0700, Juerg Haefliger wrote: > On 6/18/07, Hans de Goede <j.w.r.degoede at hhs.nl> wrote: > > As said already I'm working on the fintek 71882 driver, I've currently given it > > a tempX_input tempX_max tempX_max_hyst and a tempX_crit, with the current dyn > > chip + generic print support this results in: > > CPU: +38.0?C (high = +80.0?C, hyst = +4.0?C) sensor = transistor > > temp1_crit: +81.0?C > > > > This is because currently for max + hyst chips the first line gets printed, and > > then for chips which have a crit to a sensors_get_label_and_valid is done on > > the sensors_feature_data describing tempX_crit. Wouldn't it be better to reuse > > the main temp label and value, resulting in: > > CPU: +38.0?C (high = +80.0?C, hyst = +4.0?C) sensor = transistor > > CPU: +38.0?C (crit = +81.0?C) > > > > Maybe change the label into all whitespace / "CPU (continued)"? The current > > printout seems very wrong, but my suggested one doesn't feel right either. > > Hmm... Neither looks right to me. I can see user comments/bug reports > flying in... At least the current way has been in use for years now. "sensors" prints something like that for the LM90/ADM1032 PC87366 chips already, and I don't remember anybody complaining. OTOH, printing the same value twice seems pretty wrong. So while I agree none of these methods are ideal, the latter seems even worse to me. > Isn't there a way to make the print routine collect all the data and > print in on a single line? We could do that, the problem is that the list can get quite long. There can be up to 5 limits for each temperature input: min, max, max_hyst, crit and crit_hyst. A chip like the ADM1032 has all of them! I think that the idea with multiple lines was to make the output fit in 80 columns. If we want to stick to that, this means a maximum of two limits per line. So we could present things that way: CPU: +38.0?C (high = +80.0?C, hyst = +4.0?C, crit = +81.0?C) sensor = transistor The exact algorithm is not trivial because of hysteresis values, which we always want on the same line as the limit they correspond to. Or maybe we can have "sensors" read the COLUMNS environment variable and print things differently depending on the value, for example on multiple lines if COLUMNS=80 and all on one line if COLUMNS>80. This can quickly get complex to implement though, and might also be confusing (COLUMNS isn't exported by default.) > And our sysfs standard defines hyst to be an absolute value, not relative. True. > > Also as already "explained" in my previous question, this chip has one > > hysteresis for both max and crit, now to make the current code work I would > > need to put that twice into sysfs, which would result in to both changing if > > one gets written (I can make only one rw to make this obvious). > > That should be OK. I did something similar in the vt1211 where a > single register was exported twice and one of the attributes was > read-only. Yes, that's the way to go. -- Jean Delvare