Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding calibration code #2

Merged
merged 16 commits into from
Dec 9, 2013
Merged

Adding calibration code #2

merged 16 commits into from
Dec 9, 2013

Conversation

GM3D
Copy link
Contributor

@GM3D GM3D commented Oct 23, 2013

Hello Philipp,

I've read an article about the TEMPer device on the March issue of Linux Journal, which is based on your code.
I've given a try for it and it's working great.

However, as it is often the case with these cheap devices, my TEMPer needed some calibration to show a correct value. So I looked into your code and made some modification to include a simple linear calibration formula.

Basically TemperDevice class reads a configuration file /etc/temper.conf for calibration parameters (which are decided beforehand by other means), stores them into calib_data attribute. And when get_temperature() is called, it checks if there is a calibration data for this device ( had to add a new argument to this method to indicate what "this device" means), and if there is one, applies it to calibrate temp_c.

I'm new to Github and this is actually my first pull request, so there might be several points that should be done in better way. Any comments/suggestions or modifications will be appriciated.

Regards,
Joji Monma (GM3D on Github)

TemperDevice.__init__() reads /etc/temper.conf if it exists, and calibrates each device which has a configuration line in the file.
Format of the config file is, for example,

Device 0: scale = 0.937, offset = -0.5625

(see CONF_RE in temper.py for details).
Calibration is made with the formula

y = scale * x + offset

where x is the raw value read from the device (in celcius), y is the calibrated value, and scale and offset are read from the config file.

Since TemperDevice class object doesn't know what device number it is assigned,
an argument "id" is added to TemperDevice.get_temperature() to tell it which calibration data line to use.
@padelt
Copy link
Owner

padelt commented Oct 23, 2013

Hi Joji, thanks a lot for the effort! If I get that right you somehow determine the coefficients for calibration using some external measurement and note them in /etc/temper.conf as tuples of the form (id, scale, offset). As long as you have got only one device, this is fine. If you have more attached at the same time, you are definitely out of luck since the order in which the devices appear, cannot be reliably determined.

I have one objection with the code and that is passing the id (only) to the get_temperature method. If we somehow preassume that we know which device is which, then we should pass that value in the constructor of TemperDevice and just use the stored value. This way we can also just store the calibration data for that device instead of storing them all in each TemperDevice instance. Could you try and change that before I merge it?

All the best, Philipp

@@ -11,9 +11,9 @@ def main():

for i, dev in enumerate(devs):
readings.append({'device': i,
'temperature_c': dev.get_temperature(),
'temperature_c': dev.get_temperature(id=i),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The id should be a property of the TemperDevice-instance and should be passed on construction (defaulting to None). There should also be a note in the constructor to remind the user that the TEMPer device up until now are undistinguishable and can swap order with every reenumeration.

@GM3D
Copy link
Contributor Author

GM3D commented Oct 23, 2013

Thank you for the quick response, Philipp.

You are right, perhaps I will have to look into udev rules and libusb/python-usb code to know how the order of the devices are determined. Or maybe simply consulting to USB Bus/Device ID...mmm...

I'll try and see if I can come up with a better way.

Regards,
Joji Monma

@padelt
Copy link
Owner

padelt commented Oct 23, 2013

If you could find anything to establish an order that is (only) dependant on the physical ordering of the hubs and devices, that would be awesome. Note though that the obvious sources came up empty: https://github.com/padelt/temper-python#note-on-multiple-device-usage
From what I understand, the OS (or better: the USB stack) should be able to determine the physical structure of the bus, but I cannot find any way to make libusb spit that information out. Also, the TEMPer devices report 0 as serial number - obviously in an effort to save production cost.
-- Philipp

@GM3D
Copy link
Contributor Author

GM3D commented Oct 23, 2013

Just read your note mentioned above and tried experimenting with my RasPi.

First attempt, TEMPer in the lower USB slot on RasPi.
root@raspi:~# lsusb -t
/: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=dwc_otg/1p, 480M
|__ Port 1: Dev 2, If 0, Class=hub, Driver=hub/3p, 480M
|__ Port 1: Dev 3, If 0, Class=vend., Driver=smsc95xx, 480M
|__ Port 3: Dev 71, If 0, Class=HID, Driver=usbfs, 1.5M
|__ Port 3: Dev 71, If 1, Class=HID, Driver=usbfs, 1.5M

Second attempt. TEMPer moved to the upper USB slot.
root@raspi:~# lsusb -t
/: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=dwc_otg/1p, 480M
|__ Port 1: Dev 2, If 0, Class=hub, Driver=hub/3p, 480M
|__ Port 1: Dev 3, If 0, Class=vend., Driver=smsc95xx, 480M
|__ Port 2: Dev 73, If 0, Class=HID, Driver=usbfs, 1.5M
|__ Port 2: Dev 73, If 1, Class=HID, Driver=usbfs, 1.5M

Third attempt. Unplugged TEMPer and replugged into the same (upper) USB slot.
root@raspi:~# lsusb -t
/: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=dwc_otg/1p, 480M
|__ Port 1: Dev 2, If 0, Class=hub, Driver=hub/3p, 480M
|__ Port 1: Dev 3, If 0, Class=vend., Driver=smsc95xx, 480M
|__ Port 2: Dev 74, If 0, Class=HID, Driver=usbfs, 1.5M
|__ Port 2: Dev 74, If 1, Class=HID, Driver=usbfs, 1.5M

Yeah, confirmed. Device number can vary easily ... the only possibility that might work to physically identify this device seems to be Bus/Port/If combination. That should at least survive reboots and temporary disconnections. Of course if you move TEMPers around among multiple USB ports, you will need to reassign these numbers (in config file maybe) again though.

@padelt
Copy link
Owner

padelt commented Oct 23, 2013

You are seeing more structure than I had on a RasPi, that is encouraging! But also try to unplug and replug a device multiple times (same socket/slot/connector). In my tests on a PC, something (can't remember) changed so even the order in the tree would change.
If you find that to be stable, then the next step would be to to expose and use the information while enumerating devices in the driver.
Good luck!!
-- Philipp

George Momma and others added 9 commits October 24, 2013 22:42
TemperDevice now accepts "bus" argument in its __init__() method.
Initializer code looks upon usb sysfs directory using this bus and deviceID
information, and finds the port(or chain of ports) which this device is
plugged on. After that, it reads configuration file (/etc/temper.conf) and
uses calibration parameters found on the line with matching bus-ports.

This is needed to uniquely identify the physical location of a TEMPer device,
since only bus-ports combination can persist across reboots or connection
 losses.
… into with-calibration

Conflicts:
	temper/temper.py
Adjusted regular expression for USB bus-ports pair, so that example
line can be commented out with a preceding hash mark.
Instead of reading devpath from sysfs entry, use matched pattern for entry name itself.
Add error handling to readattr()
Fix regular expression to match sysfs entry
Merge "Usage" section of README.md to Instllation section since
temper/temper.py does not have it's own main() code.
Add some note about multiple devices in README.md
Minor spelling correction in temper/temper.conf.sample
@GM3D
Copy link
Contributor Author

GM3D commented Oct 25, 2013

Modified the way the devices are distinquished, using port numbers.

I've looked into the usbutils source code to see what lsusb is doing inside. There was a python version in there (lsusb.py), which saved time for me. From what I see, it is peeking into /sys/bus/usb/devices and reading there recursively to get the usb device tree. So I borrowed the idea from there to get the port numbers that are leading to the device with matching bus/device ID. So far the bus/port combination seems to be working and persistent enough. The cases which this scheme might not work that come int my mind are;

usb sysfs is not in /sys/bus/usb/devices. can happen on some distros. (your RasPi maybe?)
you even hotplug USB root hubs. (maybe tablet like machines with attachable keyboard/USB dock?)

But for most regular use cases hopefully this method would work.
Your comments and thoughts will be appriciated, not in a hurry at all though.

George Momma and others added 5 commits October 25, 2013 18:39
Store ports (string like "2.1.2") as an attribute so that it can be
used later for output. Modify find_ports() method accordingly.
Add -h option to temper/cli.py for usage help.
Add -p option to temper/cli.py for bus-port output.
padelt added a commit that referenced this pull request Dec 9, 2013
Adding calibration code by GM3D, thanks!
@padelt padelt merged commit bac3cf4 into padelt:master Dec 9, 2013
padelt pushed a commit that referenced this pull request Jan 5, 2016
Add "all" sensor flag support to cli.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants