Closed GoogleCodeExporter closed 8 years ago
great idea :-)
Original comment by teichsta
on 11 Mar 2013 at 9:21
your feature description seems to very feasible :-)
But, the given Java-API is IMHO not very consistent (see discussion here
http://www.tinkerunity.org/forum/index.php/topic,1444.0.html (German)).
To make the binding really nice it would be great to create a more
sophisticated API.
What do you think?
Original comment by teichsta
on 11 Mar 2013 at 9:27
(German is not the problem, English is harder for me. I'm from Germany, living
in BW.)
I like feasible approaches. They are a good starting point to get working code
and there is enough room to get even better. As long as there is no api /
configuration released refactoring should be easy.
I agree, the tinkerforge api is sometimes a bit unjavaish ;-(.
I think it would be a lot of work to wrap the whole api without loosing any
functionality and even harder to maintain this code.
Some month ago, I've made some code, which wrapped a part of tinkerforge api
with an *EMF* model. You get all these nice getters and setters and the
notification api from the generated code.
If there are limited needs to control / query the devices - what I would state
for the openhab integration - , this could be a feasible approach.
What do you think?
Original comment by theo.weiss@gmail.com
on 12 Mar 2013 at 7:21
Hallo Thomas,
a short update on this issue:
I have a rough first implementation of the binding. I have to do some cleanup
and testing, and for sure some improvements, before I push the repo to a clone
repository at google. I hope to get this finished the next weekend.
Regards,
Theo
Original comment by theo.weiss@gmail.com
on 17 Mar 2013 at 9:22
GREAT :-)
Let me know if i can assist and if your repo is pushed.
Original comment by teichsta
on 18 Mar 2013 at 6:50
Hi Thomas,
I've got a severe cold and lost the whole last week. I'll push the repo as soon
as I'm fit enough to check the current code.
Regards,
Theo
Original comment by theo.weiss@gmail.com
on 24 Mar 2013 at 9:14
ok, get well soon!
Original comment by teichsta
on 24 Mar 2013 at 9:16
Hi Thomas,
I just pushed the repo to:
https://code.google.com/r/theoweiss-tinkerforge
It's work in progress. Amongst others there are some "Heisenbugs" when
disconnecting the wifi brickd.
The configuration for now looks e.g. like this:
Number Tinkerforge_Temperature "Tinkerforge Temperature [%.1f
°C]" <temperature> (Weather_Chart) { tinkerforge="host=192.168.1.50,
port=4223, device_uid=7Dj, device_type=temperature" }
Number Tinkerforge_Humdity "Tinkerforge2 Humidity [%.1f ]" <temperature>
(Weather_Chart) { tinkerforge="host=127.0.0.1, port=4223, device_uid=b2j,
device_type=humidity" }
Regards,
Theo
Original comment by theo.weiss@gmail.com
on 26 Mar 2013 at 11:06
thank you very much!
Will have a look into it ...
Original comment by teichsta
on 26 Mar 2013 at 11:14
I pushed some updates.
Now the polling works for wifi too.
Original comment by theo.weiss@gmail.com
on 28 Mar 2013 at 2:02
Hi Theo,
i took a first look into your binding which seems to be good starting point.
I am not quite sure about the additional benefit of the wrapped TF-API? The
provided API (by Tinkerforge itself) supports a notification mechanism already
so we won't need this to move this to EMF. I had assumed the model would
generalize the Interface of the different Bricklets e.g. (Temperatur/Humidity,
etc.).
Probably i missed the point?!
Regards,
Thomas E.-E.
Original comment by teichsta
on 31 Mar 2013 at 10:53
Hi Thomas,
>Hi Theo,
>i took a first look into your binding which seems to be good starting point.
>I am not quite sure about the additional benefit of the wrapped TF-API?
>The provided API (by Tinkerforge itself) supports a notification mechanism
already
>so we won't need this to move this to EMF.
>I had assumed the model would generalize the Interface of the different
>Bricklets e.g. (Temperatur/Humidity, etc.).
>Probably i missed the point?!
I hope so ;-)
May be you haven't found the hand crafted-code inside the generated code? (yes
I'm familiar with discussions about mixing generated and hand written code.
Some posts of Ed Merks convinced me, that there are more advantages then
disadvantages when mixing code.)
I will try to explain the implementation.
In MBrickdImpl you will find the connect() method and some more private methods.
These handle the tinkerforge IPConnection, the enumeration of connected devices
and adding objects for these devices to the EMF model. Also disconnect events
are handled. Disconnecting and reconnecting a *usb* connected master brick
should work (this is done using a tinkerforge EnumerationListener).
I've also tried to handle lost connections to a brickd, but this code is
broken, because the somewhat strange behavior especially of a wifi connected
brickd (tinkerforge Connected/DisconnectedListener).
I found this forum topic:
http://www.tinkerunity.org/forum/index.php/topic,1380.msg8724.html#msg8724
The MBrickletHumidityImpl and MBrickletTemperatureImpl have the hand-crafted
methods init() and fetchSensorValue() (the implementation for the
DistanceIRBricklet is missing for now).
init() is called when a new Sensor/Bricklet is added to the model. The method
registers an appropriate listener for the observed value (e.g. a
BrickletHumidity.HumidityListener or a
BrickletTemperature.TemperatureListener). This listener updates the sensorValue
field of the object (a hysteresis value is taken into account - for now
hardcoded).
fetchSensorValue() is the generalized method, which gets and returns the
current sensor value (independent of the registered listener), by calling the
bricklet specific getter for the value (e.g. bricklet.getTemperature(),
bricklet.getHumidity()).
What the EMF model provides:
- bookkeeping of the connected tinkerforge devices:
- generalized interface to the tinkerforge devices: fetchSensorValue,
getSensorValue, EContentAdapter as generic listener to changes of sensor values.
- shared resource handling: IPConnection; only one shared ipConnection for all
devices which are connected to one brickd.
- clue between TinkerforgeBinding classes and the physical tinkerforge devices.
Description of the EMF model:
Ecosystem: the entry point to the model, knowing all MBrickds
MBrickd: holds the IPConnection to one brickd; one MBrickd for all MDevices
connected to the same host:port brickd.
abstract MDevice: superclass for all physical devices (MDevices are not in any
case identical with a concret Tinkerforge device, a MDevice could also be a
temperature or voltage sensor on a master brick).
abstract MDevice::MSensor: superclass for all devices of which provide a sensor
value
MDevice::MSensor::MBrickletHumidity: wrapper around tinkerforge BrickletHumidity
MDevice::MSensor::MBrickletTemperature: wrapper around tinkerforge
BrickletTemperature
...
MDevice::MActor: Future development: abstract MActor class - may be with a
setActorValue method - for devices, which have setters for a state (e.g. a
relay).
Model lifecycle:
TinkerForgeBinding.java:
allBindingsChanged(): a new empty Ecosystem is created. An EContentAdapter is
added.
bindingChanged(): all needed MBricks are created and connected.
MBrickd.connect(): an EnumerationListener is registered and the brickd is
forced to enumerate the devices.
MBrickd::EnumerationListener: for all found and known devices the appropriate
MSensor is created and added to the model. For removed devices the MSensor is
removed from the model.
MSensor.init(): the appropriate tinkerforge Object is constructed and the
object specific listener is added. Starting from now the senor values of the
MSensors are updated through the listeners.
(Dynamic item registry: currently the model holds all connected tinkerforge
devices (if an implementation is available), not only these configured in the
items file. This could be used in the future to register the tinkerforge items
dynamically(?). At the moment only values of configured items are propagated to
the eventbus.)
TinkerForgeBinding.java:
The execute() method is not really needed, because there are listeners
registered to the tinkerforge sensor devices and updated values are send to the
openhab bus when they occur. Explicitly calling the tinkerforge getters is
needed as workaround for the behavior of the tinkerforge api, if a the brickd
socket is shutdown: if only callbacks/listeners are used to query the devices
you would not notice the shutdown. (But there are still some things missing).
Current state:
Querying Temperature- and Humidity Bricklets should work. They may be connected
to different brickds. Connecting and disconnecting the usb connection of a
master brick should work. Restarting the brickd or restarting a wifi master
brick will enforce restarting openhab. I have to fix this!
I hope this explained some things.
New repo: https://theo.weiss@code.google.com/r/theoweiss-tinkerforge2/
I accidentally checked in generated code from the openhab core :-(( .
Furthermore may be changes to the hgignore file corrupted the repo. I checked
out a clean upstream repo and tarred the tinkerforge binding to the repo
(loosing the hg log :-( ).
I changed the output src folder for the xcore/emf model to src/main/java.
openhab:
I have some questions concerning the lifecycle of a binding:
- when is bindingChanged called?
- when is allBindingsChanged called?
- when is updated called?
Regards,
Theo
Original comment by theo.weiss@gmail.com
on 1 Apr 2013 at 7:18
Hi Thomas,
i pushed some updates to:
https://theo.weiss@code.google.com/r/theoweiss-tinkerforge2/
New supported devices:
BrickServo: switch item for demo purposes (+-90 degrees).
DualRelayBricklet: the Easter bunny brought me this nice real switch device.
But still the code is premature.
Regards,
Theo
Original comment by theo.weiss@gmail.com
on 10 Apr 2013 at 9:16
Hi Thomas,
I pushed many updates.
There is an incompatible change in the items configuration: the names for
device_type changed. You have to update your configuration. Sample
configuration can be found in demo.items in the repo.
New supported devices:
BrickletLCD20x4:
There is a example theo.rules in the repo. To write to the LCD you can send a
StringCommand with the magic prefix "TFNUM<$line$postion>". $line is the line
number starting from zero, $postion is the position in line starting from zero.
If there is no prefix the text is printed as is.
Backlight can be switched on and off.
The switch buttons are supported as SubDevice MLCD20x4Button.
BarometerTemperature:
The temperature sensor of the BrickletBarometer.
The Tinkerforge Weatherstation Kit is fully supported. Look at theo.rules for
an example.
Regards,
Theo
Original comment by theo.weiss@gmail.com
on 6 May 2013 at 7:11
Hi Theo,
thanks for this update.
While using the binding i observed very high CPU usage (at Kai's machine as
well). Haven't looked into the details. Could you have a look at that?
Regards,
Thomas E.-E.
Original comment by teichsta
on 7 May 2013 at 3:54
Hallo Thomas,
Am 07.05.2013 um 17:54 schrieb openhab@googlecode.com:
I never noticed this. I will do some investigation.
Original comment by theo.weiss@gmail.com
on 7 May 2013 at 7:19
Hi Thomas,
I've done a quick test.
The cpu usage is moderate (1.x to 4.x % on my mac mini), as long as I don't
point my browser to the openhab web site. Opening the openhab web site with the
sensor values, combined with a high CallbackPeriod (=update rate) let raise the
cpu usage very fast to 100%. I've tested it using the demo.sitemap with the
Weather_Chart (which I haven't used for some time because the page is
flickering on updates).
Removing the chart from the page leads to a moderate cpu usage again.
Is it possible that you observed the same problem?
Regards,
Theo
PS: I've implemented configuring the callbackPeriod in items.file. Just add
"callbackPeriod=100" to get 100 millis callbacks.
Supported device are all which have CallbackListeners. For now:
bricklet_humidity, bricklet_distance_ir, bricklet_temperature,
bricklet_barometer, bricklet_ambient_light
The threshold should be also configurable, but I've not tested this until now.
Original comment by theo.weiss@gmail.com
on 7 May 2013 at 8:30
Hi Thomas,
got openhab and tinkerforgeBinding working on my raspberry pi!
I've pushed some fixes for maven build. I added some debian init scripts to
distribution bundle. Don't know if it's the right place. The scripts have to be
fine tuned.
I'm working on some debugging using the MemoryAnalyser.
Regards,
Theo
Original comment by theo.weiss@gmail.com
on 9 May 2013 at 7:50
Hi Thomas,
just realized the difference between state updates and commands. may be there
is some looping in the binding. I will have a look at that this weekend.
Regards,
Theo
Original comment by theo.weiss@gmail.com
on 10 May 2013 at 8:07
Hi Thomas,
I found no obvious issues in the tinkerforge binding. May be i missed something?
Runs on my PI since two days:
23:40:33 up 2 days, 5:00, 1 user, load average: 0,11, 0,06, 0,05
I pushed some updates.
Regards,
Theo
Original comment by theo.weiss@gmail.com
on 14 May 2013 at 9:43
[deleted comment]
[deleted comment]
Original comment by teichsta
on 21 May 2013 at 9:37
Hi Thomas,
FYI:
I'm making a major refactoring by moving most configuration from items file to
openhab.cfg.
I just get it working. It looks very well until now.
Regards,
Theo
https://groups.google.com/forum/?hl=en&fromgroups#!topic/openhab/4ZjCbOUGeyE
Original comment by theo.weiss@gmail.com
on 21 May 2013 at 10:28
thanks for this update!
Original comment by teichsta
on 21 May 2013 at 10:50
Hi Thomas,
yesterday I pushed an update. It's incompatible in configuration. Current
config looks like this:
items:
Number Tinkerforge_DistanceIR "Tinkerforge5 DistanceIR [%.1f ]" {
tinkerforge="uid=6GN" }
Switch TinkerforgeServo6 "TinkerforgeServo6" {
tinkerforge="uid=6Crt5W, subid=servo6" }
Switch TinkerforgeDualRelay1 "TinkerforgeDualRelay1" {
tinkerforge="name=relay_coffee_machine" }
openhab.cfg:
tinkerforge:hosts=127.0.0.1:4223 192.168.1.50 192.168.1.104
#tinkerforge:host2=192.168.1.104
#tinkerforge:host2.port=4223
tinkerforge:distance_door.uid=6GN
tinkerforge:distance_door.type=bricklet_distance_ir
tinkerforge:distance_door.threshold=100
tinkerforge:distance_door.callbackPeriod=1000
tinkerforge:servo_living_room.uid=6Crt5W
tinkerforge:servo_living_room.type=servo
tinkerforge:servo_living_room.subid=servo0
tinkerforge:servo_living_room.velocity=65530
tinkerforge:servo_living_room.acceleration=65530
tinkerforge:hosts must be present, the other values are optional.
If they are not present, the default values will be used. All attached
tinkerforge devices will be available for openhab.
I haven't yet done testing in the deep.
Reloading openhab.cfg will - for now - break tinkerforge binding.
I think this could be fixed easily, but I'm short of time at the moment.
Regards,
Theo
Original comment by theo.weiss@gmail.com
on 25 May 2013 at 9:26
Hi Thomas,
I just removed the dependency to xbase.lib
Regards,
Theo
Original comment by theo.weiss@gmail.com
on 25 Jun 2013 at 9:08
Reloading openhab.cfg is now supported!
Original comment by theo.weiss@gmail.com
on 25 Jun 2013 at 9:14
I changed the updated method to call setProperlyConfigured. The binding will
work with 1.3-SNAPSHOT.
I think 1.2 won't work.
Original comment by theo.weiss@gmail.com
on 19 Jul 2013 at 8:56
I will start working on the support for the new "Starter Kit: Hardware Hacking".
Original comment by theo.weiss@gmail.com
on 19 Jul 2013 at 8:58
Original comment by teichsta
on 25 Jul 2013 at 10:29
1st Review is done …
Original comment by teichsta
on 30 Jul 2013 at 8:15
would be great if you could report your progress here.
Thanks, Thomas E.-E.
Original comment by teichsta
on 6 Aug 2013 at 4:28
Added lots of javadoc.
Starting work on the wiki page.
Original comment by theo.weiss@gmail.com
on 29 Aug 2013 at 8:13
added my (german) review comments below (noticed that i didn't add them to this
issue)
############################
Hi Theo,
anbei meine Kommentare zu Deinem Binding:
# generell
* die Lizenzheader sollten meiner Meinung nach unverändert bleiben, da sowieso
über ein Maven-Plugin verändert werden. Das Du der Autor bist, dokumentierst
Du ja jeweils im Author-Flag. Spätestens wenn andere Patches zum Binding
schicken, wäre die Info nicht mehr korrekt.
* generierte Klassen gehören nicht ins Repo. Im Projekt org.openhab.model.item
kannst Du sehen, wie es sein sollte.
* bitte verschiebe die Modell-Datei nach src/org.openhab.binding.tinkerforge
(ebenfalls wie im org.openhab.model.item bundle)
* Tinkerforge.jar bitte auf aktuelle Version hochziehen
* Tinkerforge.jar bitte umbenennen (lowercase starten), sodass Version
enthalten ist: tinkerforge-2.1.?.jar
* Abschnitt für Tinkerforge in der openhab_default.cfg mit der Dokumentation
aller Parameter fehlt
* mir sind die Prefixe der generierten Klasse nicht ganz klar. Für meinen
Geschmack könnten die ganzen Abkürzungen OH, M, TF weg und statt dessen ein
adäquater Name gefunden werden. Meistens ergibt sich dann ein lesbarerer Name.
Das die Klasse generiert ist, sieht man, wenn man reinschaut. Unsere Interfaces
tragen ja (Gott sei Dank :-)) auch kein 'I'. Die Prefixe sollen vermutlich der
Strukturierung dienen, was man in Java vermutlich eher mit Packages machen
würde, oder?
*
# TinkerforgeBindingProvider
* Javadoc der Klasse fehlt
* Alle Methoden müssen dokumentiert werden (gerade bei den Interfaces)
# TinkerforgeGenericBindingProvider
* Javadoc der Klasse bitte um Beispiel für gültige Konfigurationen ergänzen
(siehe andere Bindings)
* die Member DEVICE_UID, DEVICE_SUBID und DEVICE_NAME könnten gut als
Enumeration abgebildet werden
* Z60 > Statement scheint mir überflüssig
* validateItemType() > auskommentierten Code bitte entfernen
* Z100 > hier könnte man auch die "neue" Form der For-Schleife nehmen: for
(String token : tokens) {} da der index ja nicht weiter benötigt wird
* Z107 > hier sollte noch erwähnt werden, warum das Format invalid ist: "must
consist of ..."
# TinkerforgeGenericBindingProvider.TinkerforgeBindingConfig
* Implementierung von check() scheint mir ziemlich "schräg" dafür dass eine
BCPE geworfen werden soll, wenn 'uid' null ist
# LoggerConstants
* bitte in eine Enumeration umwandeln
# TinkerforgeBinding
* Javadoc der Klasse fehlt
* TODOs entfernen
* auskommentierten Code entfernen
* logger.debug für reine Methodencalls entfernen
* mindestens public und protected Methoden dokumentieren > besser aber auch die
private da hier einiges an "Musik" spielt
* bitte auch Conditionals mit einer Zeile klammern, um die Lesbarkeit zu
erhöhen!
* Z95-98 CfgKeys bitte in eine Emueration umwandeln
* Z99-105 TypeString bitte in eine Enumeration umwandeln
* Z134 "More cleanup is needed" > wenn das so ist, solltest Du das auch machen
:-)
* Z139 müsste der Aufruf der Methode nicht synchronized sein?
* Z148 müsste der Aufruf der Methode nicht synchronized sein?
* Z167 hier könnte man auch ein LogStatement spendieren um anzuzeigen, dass
man das BrickD aus dem Cache gewählt hat
* Z176 LogStatement sollte im if-Block debug sein und dann Anzeigen, welches
Devices geadded wurde. Im else-Zweig dürfte auch das Level Trace reichen
* Z192 Methode bitte unbedingt aufsplitten. Jede der if
(notification.getEventType() == Notification.ADD) { -Blöcke verdient aufgrund
seiner schieren länge eine eigene Methode. Vielleicht bringt es auch etwas die
Verschachtelung (Notifier vs. EventType zu drehen, um Codedopplung zu vermeiden
* Z310 auch die Methode ließe sich prima splitten: processSensorValues() und
processActorValues() und dabei würde dann auch gleich wieder dieses 'TF'
entfallen
* Z312 und weitere > der neue Value sollte ruhig mit in das LogStatement,
immerhin liegt er ja passen vor
* Z315,328 Du lässt dir den Wert zunächst als double geben, um ihn dann in
einen String zu wandeln. Wieso holst Du nicht gleich den Wert mit
getStringValue() ?
* Z367 Name des Items sollte in das LogStatement
* bitte massive CodeDopplung zwischen processTFValues() und execute() auflösen
* wird execute() überhaupt benötigt? Gibt es Devices deren Wert zyklisch
abgefragt wird?
* der Block Z482-492 taucht sehr häufig auf und sollte refaktorisiert werden
* Z507-513 der Block gehört in die internalReceiveUpdate(), da hier kein
Command verschickt werden soll, sondern der Wert Matrix aktualisiert wird,
richtig?
* Z648 hier beginnt der gleiche Block wie in Z554 bitte Dopplung durch anderen
Schnitt der Methode auflösen
* Z724 bitte in Konstante wandeln
# MBrickdImpl
* Z482 scheint überflüssig
* Z485 warum wird nicht auf Disconnect-Ereignisse gelauscht?
* Z496 warum wird der Verbindungsaufbau in einem eigenen Thread ausgeführt?
* Z550 Methode wird nicht mehr benutzt >
So, dass war es erstmal von meiner Seite. Insgesamt macht das Binding für mich
einen guten Eindruck. Dennoch stecken in TinkerforgeBinding noch eine Menge
CodeDubletten, die ich gerne entfernen würde. Bist Du mit der Wiki-Seite
klargekommen? Denkst Du, Du kann die Anmerkungen noch vor Deinem Urlaub
einarbeiten?
Gruß,
Thomas E.-E.
Original comment by teichsta
on 1 Sep 2013 at 1:10
Merged into default branch (see
http://code.google.com/p/openhab/source/detail?r=59e9d0f5dbd8a9c2b7e94a1a251bedf
da1282b09).
Thanks Theo for this contribution!
Original comment by teichsta
on 1 Sep 2013 at 1:36
Got a maven warning while compiling default. It seems that there is no version
specified for org.vafer in pom.xml.
Was this intended?
[WARNING]
[WARNING] Some problems were encountered while building the effective model for
org.openhab.binding:org.openhab.binding.
tinkerforge:eclipse-plugin:1.3.0-SNAPSHOT
[WARNING] 'build.plugins.plugin.version' for org.vafer:jdeb is missing. @ line
29, column 14
[WARNING]
[WARNING] It is highly recommended to fix these problems because they threaten
the stability of your build.
[WARNING]
[WARNING] For this reason, future Maven versions might no longer support
building such malformed projects.
[WARNING]
Original comment by jwsp...@gmail.com
on 1 Sep 2013 at 10:56
Thomas fixed it in revision: 95babee94aaa
Original comment by theo.weiss@gmail.com
on 13 Sep 2013 at 7:12
Hi Thomas,
can you tell me on which revision of my clone the checkin to the default branch
is based?
I try to investigate if there are outstanding bugfixes.
Regards,
Theo
Original comment by theo.weiss@gmail.com
on 13 Sep 2013 at 7:14
#37 is already fixed in default
#39 to be honest: no clue :-)
Original comment by teichsta
on 13 Sep 2013 at 7:24
#39 I figured it out. No *known* bugs for 1.3.1 until now.
Original comment by theo.weiss@gmail.com
on 13 Sep 2013 at 8:00
cool, good news :-)
Original comment by teichsta
on 13 Sep 2013 at 8:01
An update for things I've been working on the last weeks:
New devices:
* Bricklet Industrial Quad Relay
* Bricklet Industrial Digital In 4
* Bricklet IO-16
Incompatible Changes:
* LCDBacklight is a sub device of LCD20x4 Bricklet (items file must be changed)
* LCD20x4Button posts an update not a command anymore (rules must be changed)
* IndustrialQuadRelay sub id numbering now starts from zero (items file must be changed)
Bugfixes:
* Missing updates of Items if a Tinkerforge Device is referenced in several Items
Other changes:
* support for serveral Item types
* NumberItem
* SwitchItem
* ContactItem
Structural changes:
* support for digital sensors and actuators in the model
* all updates to the event bus use one method: TinkerforeBinding.postUpdate()
* on bindingChanged events all item values are updated
* when a device is enabled it fetches the current state from TF device and updates the model value, in fact updates are send to the event bus
* handle disconnected brickds
* on startup make retries every second
* when running use the Tinkerforge auto reconnect feature
* added a Readme.md to the bundle to document the changes
Outstanding changes: implement all requested changes from the code review
Currently working on: configuration validation for the values from openhab.cfg
I'm preparing a pull request, but I've to figure out how this works :-(((.
Regards,
Theo
Original comment by theo.weiss@gmail.com
on 16 Nov 2013 at 9:03
updated Tinkerforge API to 2.0.12
Original comment by theo.weiss@gmail.com
on 16 Nov 2013 at 9:25
Original issue reported on code.google.com by
theo.weiss@gmail.com
on 11 Mar 2013 at 9:11