Bug#712841: A bit late, but still an issue...

[Martin Michlmayr]

* Axel Sommerfeldt  [2018-04-22 12:01]:
> And after setting "has_fan" to "false":

Ok, thanks for testing!

I hope Ian can apply this upstream and release a new Debian package.
It probably makes sense to update debian/NEWS (see proposed patch in
the attachment).  debian/README.Debian is also quite out of date.

> > Maybe, although the fanless devices are designed to work without a
> > fan so this shouldn't happen.  But what would you consider "too
> > hot"?
> 
> I have no idea what max. temperature the qnap can handle. Maybe 70?
> Maybe it should not beep but at least change the status of the
> status led? Or change the status led at 70 and beep at 75? (But this
> is only clueless guessing of mine.)

I don't really know.  Maybe Ian has some ideas.
-- 
Martin Michlmayr
http://www.cyrius.com/
diff --git a/debian/NEWS b/debian/NEWS
index cddd852..eb51b70 100644
--- a/debian/NEWS
+++ b/debian/NEWS
@@ -1,3 +1,11 @@
+qcontrol (0.5.6-1) unstable; urgency=low
+
+  * QNAP devices without fan handled correctly.  The config file now
+handels fanless QNAP devices correctly when `has_fan` is set to
+`false`.
+
+ -- Martin Michlmayr   Mon, 23 Apr 2018 11:58:35 +0200
+
 qcontrol (0.4.2-2) unstable; urgency=low
 
   * Input device changed

Bug#712841: A bit late, but still an issue...

[Axel Sommerfeldt]

On Thu, Apr 19, 2018, at 12:47, Martin Michlmayr wrote:
> * Axel Sommerfeldt  [2018-04-16 21:25]:

> Based on Ian's comments, I added a has_fan to the configuration file.
> I believe the attached patch should do the trick.
> 
> Axel, can you test this (after restoring the original config).

Looks good to me. Right after patching:

Apr 22 11:40:26 qnap systemd[1]: Started LSB: Start qcontrol daemon.
Apr 22 11:40:26 qnap qcontrol[338]: qcontrol 0.5.5 daemon starting.
Apr 22 11:40:26 qnap qcontrol[338]: Register evdev on 
/dev/input/by-path/platform-gpio_keys-event
Apr 22 11:40:26 qnap qcontrol[338]: confdir: loading from /etc/qcontrol.d...
Apr 22 11:40:26 qnap qcontrol[338]: System status: start
Apr 22 11:40:30 qnap qcontrol[338]: ts219: temperature 55
Apr 22 11:40:30 qnap qcontrol[338]: ts219: temperature 55 setting fan to "high"
Apr 22 11:40:37 qnap qcontrol[338]: ts219: fan error
Apr 22 11:41:27 qnap qcontrol[338]: ts219: fan error
Apr 22 11:42:17 qnap qcontrol[338]: ts219: fan error

And after setting "has_fan" to "false":

Apr 22 11:44:26 qnap systemd[1]: Starting LSB: Start qcontrol daemon...
Apr 22 11:44:26 qnap qcontrold[323]: Starting qcontrol daemon: qcontrol.
Apr 22 11:44:26 qnap systemd[1]: Started LSB: Start qcontrol daemon.
Apr 22 11:44:26 qnap qcontrol[333]: qcontrol 0.5.5 daemon starting.
Apr 22 11:44:26 qnap qcontrol[333]: Register evdev on 
/dev/input/by-path/platform-gpio_keys-event
Apr 22 11:44:26 qnap qcontrol[333]: confdir: loading from /etc/qcontrol.d...
Apr 22 11:44:26 qnap qcontrol[333]: System status: start
Apr 22 11:44:30 qnap qcontrol[333]: ts219: temperature 55

> > the temperature gets hot and make a beep (and a red blinking status
> > led) if it gets too hot!?
> 
> Maybe, although the fanless devices are designed to work without a fan
> so this shouldn't happen.  But what would you consider "too hot"?

I have no idea what max. temperature the qnap can handle. Maybe 70? Maybe it 
should not beep but at least change the status of the status led? Or change the 
status led at 70 and beep at 75? (But this is only clueless guessing of mine.)

Bug#712841: A bit late, but still an issue...

[Martin Michlmayr]

* Axel Sommerfeldt  [2018-04-16 21:25]:
> 1) Done
> 2) "qcontrol fanspeed stop" makes the beeping stop

Thanks for confirming this!

> Creating /etc/qcontrol.d/90-no-fan.conf with the content above makes the 
> beeping stop, too.

Great!

Based on Ian's comments, I added a has_fan to the configuration file.
I believe the attached patch should do the trick.

Axel, can you test this (after restoring the original config).

> Maybe it's a good idea to overwrite the temp() function in
> /etc/qcontrol.d/90-no-fan.conf, too? It could create a log entry if

logtemp() is still being called, so you will get temperate logs.

> the temperature gets hot and make a beep (and a red blinking status
> led) if it gets too hot!?

Maybe, although the fanless devices are designed to work without a fan
so this shouldn't happen.  But what would you consider "too hot"?
Maybe thsi should be in a config variable.

-- 
Martin Michlmayr
http://www.cyrius.com/
diff --git a/examples/ts209.lua b/examples/ts209.lua
index da301a5..51991c8 100644
--- a/examples/ts209.lua
+++ b/examples/ts209.lua
@@ -37,6 +37,13 @@ register("system-status")
 
 -- Set to "false" to suppress the sounding of the buzzer
 buzzer = true
+-- Set to "false" if your device doesn't have a fan (TS-109 and TS-109 II)
+has_fan = true
+
+-- Turn off fan if there is no fan to avoid fan_error() being called
+if not has_fan then
+	piccmd("fanspeed", "stop")
+end
 
 function system_status( status )
 	logprint("System status: "..status)
@@ -88,6 +95,11 @@ end
 last_fan_setting = nil
 
 function setfan( speed )
+	-- Do nothing if there's no fan
+	if not has_fan then
+		return
+	end
+
 	if ( ( not last_fan_setting ) or
 	 ( last_fan_setting ~= speed ) ) then
 		logprint(string.format("ts209: setting fan to \"%s\"", speed))
diff --git a/examples/ts219.lua b/examples/ts219.lua
index ab28d93..97d9b8f 100644
--- a/examples/ts219.lua
+++ b/examples/ts219.lua
@@ -38,6 +38,14 @@ register("system-status")
 -- Set to "false" to suppress the sounding of the buzzer
 buzzer = true
 
+-- Set to "false" if your device doesn't have a fan (TS-119 and HS-210)
+has_fan = true
+
+-- Turn off fan if there is no fan to avoid fan_error() being called
+if not has_fan then
+	piccmd("fanspeed", "stop")
+end
+
 function system_status( status )
 	logprint("System status: "..status)
 	if status == "start" then
@@ -128,6 +136,11 @@ end
 last_fan_setting = nil
 
 function setfan( temp, speed )
+	-- Do nothing if there's no fan
+	if not has_fan then
+		return
+	end
+
 	if ( ( not last_fan_setting ) or
 	 ( last_fan_setting ~= speed ) ) then
 		logprint(string.format("ts219: temperature %d setting fan to \"%s\"", temp, speed))

Bug#712841: A bit late, but still an issue...

[Axel Sommerfeldt]

On Mon, Apr 16, 2018, at 20:12, Martin Michlmayr wrote:

> Axel, can you restore the original config and 1) see if you get the beeps
> and 2) whether running
> 
> qcontrol fanspeed stop
> 
> makes them stop. (change setfan in the config to do nothing, otherwise
> the fan will be turned on again)

1) Done
2) "qcontrol fanspeed stop" makes the beeping stop

> Do you know if /etc/default is Debian-specific or generic?

It's not part of the Filesystem Hierarchy Standard (3.0) but at least I can 
confirm that this directory exists in CentOS and Fedora, too, containing the 
files "grub" and "useradd".

> As a workaround, until this is fixed, users can create a file
> /etc/qcontrol.d/90_no_fan with:
> 
> function fan_error(  )
> end
> 
> function setfan( temp, speed )
> end
> 
> I believe this should work but is completely untested. (Alex,
> Christian, maybe you can test after restoring the original config.)

Creating /etc/qcontrol.d/90-no-fan.conf with the content above makes the 
beeping stop, too.

Log:
Apr 16 21:08:59 qnap-backup qcontrol[330]: confdir: loading from 
/etc/qcontrol.d...
Apr 16 21:08:59 qnap-backup qcontrol[330]: confdir: including 
/etc/qcontrol.d/90-no-fan.conf

Maybe it's a good idea to overwrite the temp() function in 
/etc/qcontrol.d/90-no-fan.conf, too? It could create a log entry if the 
temperature gets hot and make a beep (and a red blinking status led) if it gets 
too hot!?

Bug#712841: A bit late, but still an issue...

[Martin Michlmayr]

* Ian Campbell  [2018-04-15 15:33]:
> > Ian, do you think it's worth asking the release team if they'd allow
> > an update for this in stable?
> 
> I think it is premature to consider that question without a specific
> fix in unstable to be backported in order to make the argument/decision
> based on it.

Yes, absolutely.  Of course we have to fix it in unstable first.

> > Getting the right device from /etc/mtdblock5 is easy, but I've no 
> > idea how to change the config based on that.
> 
> I think you mean /dev/mtdblock5? While mounting that to extract the

/dev/, yes.  But I agree with your concerns.

Anyway, after reading the bug log again and looking at the files,
here's what I think:

Devices affected:

* TS-109
* TS-109 II
* TS-119
* HS-210

The first two are ts-209 devices from the POV of qcontrol and the latter
two ts-219.

We have two problems:

1) fan_error() produces a beep.  Previous discussion suggests that turning
the fan off will make the fan_error() calls go away on devices without a
fan (at least on the TS-109).

Axel, can you restore the original config and 1) see if you get the beeps
and 2) whether running

qcontrol fanspeed stop

makes them stop. (change setfan in the config to do nothing, otherwise
the fan will be turned on again)

2) We have temp_low()/temp_high() on ts-209 and temp() on ts-219 which
regulates the temperature by calling setfan().

So imho the easiest fix would be a) for the init script to run `qcontrol
fanspeed stop` if HAS_FAN=no and for setfan() to be modified to do nothing
if HAS_FAN=no.

It would be up to users to add HAS_FAN=no to /dev/default/qcontrol

Ian, does that sound reasonable?

Do you know if /etc/default is Debian-specific or generic?  I'm just
wondering if the qcontrol configs should read from /etc/default/qcontrol
(Google suggests Red Hat and SUSE have it too, so I guess that should
be ok.)

As a workaround, until this is fixed, users can create a file
/etc/qcontrol.d/90_no_fan with:

function fan_error(  )
end

function setfan( temp, speed )
end

I believe this should work but is completely untested. (Alex,
Christian, maybe you can test after restoring the original config.)

-- 
Martin Michlmayr
http://www.cyrius.com/

Bug#712841: A bit late, but still an issue...

[Ian Campbell]

On Mon, 2018-04-16 at 20:12 +0200, Martin Michlmayr wrote:
> Do you know if /etc/default is Debian-specific or generic?  I'm just
> wondering if the qcontrol configs should read from /etc/default/qcontrol
> (Google suggests Red Hat and SUSE have it too, so I guess that should
> be ok.)

They have a different name/path for it IIRC.

I don't think it's necessary to indirect via /etc/default anyway. Just
write the variable directly into the appropriate /etc/qcontrol/foo.conf
files and have necessary conditionals in there. I believe you can also
call the fan stop function from the config file, perhaps from the
system init module callback or just do it at the top level.

Ian.

Bug#712841: A bit late, but still an issue...

[Martin Michlmayr]

* Martin Michlmayr  [2018-04-16 20:12]:
> As a workaround, until this is fixed, users can create a file
> /etc/qcontrol.d/90_no_fan with:

It needs a .conf extension, so e.g. /etc/qcontrol.d/90-no-fan.conf

-- 
Martin Michlmayr
http://www.cyrius.com/

Bug#712841: A bit late, but still an issue...

[Ian Campbell]

On Sun, 2018-04-15 at 15:33 +0100, Ian Campbell wrote:
> That said, if there are modifications which can be made to the existing
> config file installed on this variant which would minimise the
> modifications i.e. by adding a simple "has_a_fan = true" which affected
> people can set to false then that is something to consider doing I
> think.

Or if it differs by too much we could install a /etc/qcontrol/ts1xx.lua
file but not attempt to automatically link to it, so people can just
flip the /etc/qcontrol.conf symlink themselves.

Ian.

Bug#712841: A bit late, but still an issue...

[Martin Michlmayr]

* Axel Sommerfeldt  [2018-04-14 22:07]:
> Just a reminder that this bug is still a thing. And yes, the model can be 
> found in uLinux.conf:

Ian, do you think it's worth asking the release team if they'd allow
an update for this in stable?  Since stretch is likely to be the last
version to support these devices, it might be worth it.  OTOH, since
it's easy to fix by users, it might not be worth pushing more code in.

Getting the right device from /etc/mtdblock5 is easy, but I've no idea
how to change the config based on that.

Maybe the right thing to do would be to put an update into stable
documenting this issue properly.  This shouldn't be too controversial
for the stable release team.

Ian, it seems the git repo is out of date (has 0.5.5-2 while unstable
has 0.5.5-3).  The VCS headers are out of date too now with Salsa.

And debian/README.Debian says there's no daemon mode...

(My web site doesn't mention this issue either.)

> BTW: Martin, thanks a lot for all the work. Having Debian on a qnap really 
> rocks.

You're welcome.  Ian Campbell has been doing all of the qcontrol work
and I'm not doing much apart from responding to some email. :/

-- 
Martin Michlmayr
http://www.cyrius.com/

Bug#712841: A bit late, but still an issue...

[Ian Campbell]

On Sun, 2018-04-15 at 16:03 +0200, Martin Michlmayr wrote:
> * Axel Sommerfeldt  [2018-04-14 22:07]:
> > Just a reminder that this bug is still a thing. And yes, the model
> can be found in uLinux.conf:
> 
> Ian, do you think it's worth asking the release team if they'd allow
> an update for this in stable?

I think it is premature to consider that question without a specific
fix in unstable to be backported in order to make the argument/decision
based on it.

>   Since stretch is likely to be the last
> version to support these devices, it might be worth it.  OTOH, since
> it's easy to fix by users, it might not be worth pushing more code
> in.

I think /etc/qcontrol.conf is a conffile for a good reason and folks
can and should edit it to support their specific system if need be
without feeling like they are doing something "wrong".

That said, if there are modifications which can be made to the existing
config file installed on this variant which would minimise the
modifications i.e. by adding a simple "has_a_fan = true" which affected
people can set to false then that is something to consider doing I
think.

> Getting the right device from /etc/mtdblock5 is easy, but I've no 
> idea how to change the config based on that.

I think you mean /dev/mtdblock5? While mounting that to extract the
configuration might in theory be "easy" I think it is a whole new set
of code which would now need to be written, debugged etc for qcontrol,
which doesn't currently do anything as invasive as mounting or touching
devices which are not "owned" in any sense by the Debian installation.
I'm not even sure it would be within policy for a package to do that
sort of thing in a postinst and I'd certainly be very wary of it going
wrong.

I don't personally have time to work on that (nor a device to test with
in any case) but I'd be happy to look at a tested and well documented
patch.

> Ian, it seems the git repo is out of date (has 0.5.5-2 while unstable
> has 0.5.5-3).  The VCS headers are out of date too now with Salsa.

The repo on salsa is up to date I believe, please let me know if not.

I was waiting to have some other reason to upload before fixing the Vcs
fields. The redirector is working for some but not all of them (I
forget which of http and git is working vs broken).

Ian.

Bug#712841: A bit late, but still an issue...

[Axel Sommerfeldt]

Just a reminder that this bug is still a thing. And yes, the model can be found 
in uLinux.conf:

root@qnap119:~# mount -o ro -t ext2 /dev/mtdblock5 /mnt
root@qnap119:~# more /mnt/uLinux.conf 
[System]
Model = TS-119
Internal Model = TS-119
...

BTW: Martin, thanks a lot for all the work. Having Debian on a qnap really 
rocks.

Bug#712841: A bit late, but still an issue...

[Martin Michlmayr]

* Christian Mueller  [2016-05-10 18:57]:
> That would be great. You mean customise.conf, correct? Anything I can do to
> help?

No, I meant uLinux.conf.  There should be a [System] stanza with
information such as

Model = TS-219
Internal Model = TS-219

-- 
Martin Michlmayr
http://www.cyrius.com/

Bug#712841: A bit late, but still an issue...

[Christian Mueller]

That would be great. You mean customise.conf, correct? Anything I can do 
to help?


Thanks,
--Christian

P.S.: Thanks for all the effort you put into this - I'm using Debian on 
ARM devices for my mail/DNS/VPN/... server(s) since the old NSLU2 days 
and it all started on on cyrius.com :)


On 05/05/2016 10:59 PM, Martin Michlmayr wrote:

* Christian Mueller  [2016-04-29 22:20]:

Long story cut short: if there is a way to identify TS-119 vs. TS-219
hardware, a fix would be very welcome.

There's a configuration file on /dev/mtdblock5 which contains the
device type.  I'm starting to think the easiest solution would be to
mount that device, check the device type and configure qcontrol
accordingly.

Bug#712841: A bit late, but still an issue...

[Martin Michlmayr]

* Christian Mueller  [2016-04-29 22:20]:
> Long story cut short: if there is a way to identify TS-119 vs. TS-219
> hardware, a fix would be very welcome.

There's a configuration file on /dev/mtdblock5 which contains the
device type.  I'm starting to think the easiest solution would be to
mount that device, check the device type and configure qcontrol
accordingly.

-- 
Martin Michlmayr
http://www.cyrius.com/

Bug#712841: A bit late, but still an issue...

[Christian Mueller]

Hi all,

I've just upgraded my trusty and apparently indestructable TS-119 to 
Jessie and stumbled over this bug. I've simply commented-out the line 
that increases the fan error count and the beeping has stopped but I see 
log entries which show qcontrol trying to set fan speeds due to 
temperature measurements.


Long story cut short: if there is a way to identify TS-119 vs. TS-219 
hardware, a fix would be very welcome. If not, I guess an option in 
/etc/default/qcontrol to simply disable any kind of thermal management 
involving fans would be nice, too (overtemp handling excluded, of course)


Thanks,
--Christian