[pixometer] Pixometer Binding initial contribution

[Confectrician]

Pixometer Binding

This is the initial contribution of the Pixometer binding, which can connect to the pixometer.io API.
Pixometer.io provides an interface for managing your meter (gas, electricity, water) through web.
Measurements are uploaded with optical image recognition through their app (Info).
I am using this service, since i want to keep track of my meter devices and their measurements,
but i am not able to change anything in my wiring structure (rental object).
Additionally pixometer.io is free in the basic plan which lets you manage up to five meter devices.

Community Thread

Features

• Connect the pixometer api with a bridge thing
• Connect your meter devices through a meter thing.

To-Do

• Add Units of Measurement
• Maybe split meter thing up depending on UoM integration
• Proofread and improve Readme
• Testing

[openhab-bot]

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/pixometer-binding-anyone-interested/37046/5

[Confectrician]

Hey @wborn,

Do you know if travis caches some stuff?
I repeatedly get this error:
https://travis-ci.org/openhab/openhab2-addons/builds/449284229#L2320

But if have already fixed the author tag some commits ago, which is weird.

[wborn]

Do you know if travis caches some stuff?

It shouldn't cache anything. Though we've seen it happen before (https://github.com/openhab/openhab2-addons/pull/16#issuecomment-302731471). If it doesn't get resolved you could try closing this PR and create a new one. :-/

[Confectrician]

I will continue my work and do that as a last step if it doesn't get resolved.

Thanks for your answer.

[martinvw]

@Confectrician hereby your SAT report and the reason the build is failing :-)

[INFO] --- sat-plugin:0.5.0:report (default) @ org.openhab.binding.pixometer ---
[INFO] Individual report appended to summary report.
[ERROR] Code Analysis Tool has found: 
 1 error(s)! 
 3 warning(s) 
 24 info(s)
[WARNING] .binding.pixometer/META-INF/MANIFEST.MF:[11]
The exported package `org.openhab.binding.pixometer.handler` is not imported
[WARNING] org.openhab.binding.pixometer.handler.ApiserviceHandler.java:[138]
Avoid throwing raw exception types.
[WARNING] org.openhab.binding.pixometer.handler.MeterHandler.java:[167]
Avoid throwing raw exception types.
[ERROR] org.openhab.binding.pixometer.internal.config.ImageMeta.java:[23]
An author tag is missing

[Confectrician]

Thanks @martinvw, but the imagemeta.java file has an author tag since several commits.

See discussion with @wborn above.

[Confectrician]

My eclipse environment doesn't start the debugging atm.
I will do a fresh install and go on with testing afterwards.

Since there will be some delay for a firs review, i am removing the WIP now.

Thanks to @martinvw for the hint with that misplaced author tag.
On beer on my bill @ next smarthome day. 😃

[Hilbrand]

I've made some comments 🙂 In general it looks fine. Most are some general practice issues and typical openHAB conventions.

One additional comment you need to add you binding to the features/openhab-addons/src/main/feature/feature.xml file

[Hilbrand]

This pull request needs to update the license due to the license change of the project to Eclipse version 2. See this tutorial on how to update the license: https://community.openhab.org/t/tutorial-update-license-to-epl-v2-0-of-your-binding-in-development-or-pull-request/65088/4

[J-N-K]

@Confectrician What is the status here? It looks like it needs the bnd migration. Are all other comments done?

[Confectrician]

Hey @J-N-K,

I don't think that all other comments are solved already.
I plan to do the bnd Migration in the nearer future, bit did not have spare time yet.

Also there seems to be a problem with the token obtaining currently, as there is an errorr report about it in the forum.
I would like to solve those problem in one run.

[TravisBuddy]

Travis tests were successful

Hey @Confectrician,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

[Confectrician]

OK i have done the bnd migration and the epl license changes now and did some minor fixes to make the binding build again.

I am not sure about the jenkins error and will now adress the open review comments.
Edit: I think the jenkins stuff is not related to this pr, when i take a look at the build history.^^

[Confectrician]

Hey @Hilbrand

One additional comment you need to add you binding to the features/openhab-addons/src/main/feature/feature.xml file

I think this one is outdated, right?

[Hilbrand]

Yes that is outdated. There is now a feature file in the binding

[Confectrician]

[ERROR] org.openhab.binding.pixometer.internal.serializer.CustomReadingInstanceDeserializer.java:[104]
Avoid using equals() to compare against null

So what is the prefered way of comparing a string against null?
A == didn't work out and also seems to be poorer way of comparing.
-> https://stackoverflow.com/questions/767372/string-equals-versus

[kaikreuzer]

Suggested change

	return (s.isEmpty() || s.equals(null) || s.equals("null"));
	return (s==null || s.isEmpty() || s.equals("null"));

[kaikreuzer]

This should be the right way. Checking for null is clearly done with ==, you just have to make sure it is in the correct order.

[Confectrician]

Maybe i didn't try the combination of s==nullwith the other "empty" conditions.
I tried many combinations so maybe this one was lost. 🤷‍♂

[TravisBuddy]

Travis tests were successful

Hey @Confectrician,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

[Confectrician]

Travis is fine, but DenonMarantz seems to have broken Jenkins now.

[kaikreuzer]

Thanks, looks pretty good to me now! Just a last 1.5 very minor comments.

[kaikreuzer]

This looks like a tab instead of a space to me. Does the SAT check pass?

[Confectrician]

It was indeed a tab alltough i didn't recognize any complains from SAT check.
I had problems with my thing xml files before and SAT reported errors immediately.
Strange. 🤷‍♂

[kaikreuzer]

I am not fully sure, if the framework validates the value or if you should better check it here to be >=60.
By the way, one hours seems incredibly long for a minimal refresh rate. Is there a request limit on the API that makes this necessary?

[Confectrician]

By the way, one hours seems incredibly long for a minimal refresh rate. Is there a request limit on the API that makes this necessary?

Nope, i have seen no need in requesting their API unnecessary.
Those values are based on my usage of the pixometer app and i try to get one meter reading per month per meter.
With this background 60 minutes seemed "fast enough" for me.

I am not fully sure, if the framework validates the value or if you should better check it here to be >=60.

I am pretty sure that i had to manually edit the thing xml to set a lower period for debugging, so it seems that ther is some kind of validation ongoing.

[TravisBuddy]

Travis tests were successful

Hey @Confectrician,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

[kaikreuzer]

Thanks! Let see what Jenkins says and we can merge!

[kaikreuzer]

@Hilbrand You still have a "changes requested" review, so if you are happy, please approve.

[Confectrician]

Thanks for your comments, requests and tips. 🙂
This was a long journey for me, but i learned many things.