reyem / openhab2-addons

Add-ons for openHAB 2.x
Eclipse Public License 2.0
3 stars 1 forks source link

Robonect - first thoughts #1

Closed itn3rd77 closed 7 years ago

itn3rd77 commented 7 years ago

Hi Marco,

I managed to get the preview of your Robonect binding up and running. Here are my first thoughts I like to discuss with you:

WLAN Signal is defined as String inside thing-types.xml. All other bindings I have looked at define any kind of signal strength as Number. It's working fine for just showing within UI with String type but I don't know the implications later on. I think if you want to use the WLAN signal inside rules for comparison e.g. <= -60 we will get into trouble.

Mower mode and Mower Status are directly translated into meaningful strings. As most users are using Transforming to convert technical values into human readable ones you could just stick with the numbers. In either case you have to describe the possible values for each channel. Maybe it´s just a matter of taste but I would prefer the technical values and leave it up to the user to map them to what ever he wants. :-)

Next timer date, Next timer time, Next timer unix timestamp are really hard to deal with and don't give the user any real benefit in that kind of form. I would like to suggest to remove those three definitions and replace them by a single channel e.g. NextTimer with type DateTime. What do you think?

Hope it`s OK for you I am using the issue tracker. If you prefer any other kind of communication let me know!

Keep up!

Regards Ingo

reyem commented 7 years ago

Hey Ingo, Thank you for the testing and valuable feedback.

WLAN Signal is defined as String inside thing-types.xml. All other bindings I have looked at define any kind of signal strength as Number.

Definitely agree. I will change this.

Mower mode and Mower Status are directly translated into meaningful strings. As most users are using Transforming to convert technical values into human readable ones you could just stick with the numbers

Here I'm not so sure. I prefer enums over integers for such things in Java code as it limits the possibilities and also makes code much more readable. Although It would be easy to expose the int values behind as well. In the java code I defined them as enum values with ints anyways... I will think about it... Maybe we can create a sample Rule based on Status in two versions to get a base for a decision. I assume maps can also be used on String values, can't they?

Next timer date, Next timer time, Next timer unix timestamp are really hard to deal with and don't give the user any real benefit in that kind of form. I would like to suggest to remove those three definitions and replace them by a single channel e.g. NextTimer with type DateTime. What do you think?

I Agree that in the current form it does not make much sense. I simply expose the raw response from the API. Your proposal improves it definitely. I was also thinking in actually implementing the timer api which would allow to set timers as well. So maybe it can be combined somehow in a better way. http://www.robonect.de/viewtopic.php?f=11&t=108

Hope it`s OK for you I am using the issue tracker. If you prefer any other kind of communication let me know!

Yes, that is definitely the preferred way! perfect. I may split it into several issue though, but this is the most straightforward way to keep things sorted.

Cheers Marco

reyem commented 7 years ago

@itheiss,

finally I changed:

The mode I left as a string so far. There is an issue to be sorted out first. The status is used for retrieving and setting the status. However, there are the artificial modes "EOD" and "JOB". These are actually not modes, but setting these modes actually triggers an action. Now I wonder what you think about

Change mode to just support AUTO, HOME, MAN, (DEMO!?) Introducing switches for JOB and EOD. The challenge here is the switch state. I see following options: a) On a switch ON command send the JOB respectively EOD command to the mower and update the switch state immediately to OFF again

b) On a switch ON command send the JOB respectively EOD command to the mower and try to maintain a switch state in the binding. But What is a reliable combination of conditions to determine if a JOB is done or EOD is over?

EOD: Daytime over. save the time stamp when receiving the EOD and set it to off when midnight is passed. But what if the user disables EOD? How to put the mower out of EOD state?

JOB: based on the start/end/duration the theoretical job time can be calculated. Now in combination with a status change the OFF event could be determined. But again, what if the user sends an OFF to a running JOB? Stop the mower? send it home?

Any thoughts?

itn3rd77 commented 7 years ago

@reyem Thanks so far! Maybe a little state diagram can help us to see clear. Give me some time to think about it. Will come up with an proposal.

itn3rd77 commented 7 years ago

Ok here are my thoughts and what I have observed.

I would suggest to separate the status/mode and the actual commands. You asking what the hell I am talking about? Here's what's in my mind.

We have four official status/modes AUTO, HOME, MANUAL and DEMO (don't know what that means) and a number of commands AUTO, HOME, MANUAL, START, STOP. After analyzing the command EOD I realized this is in fact a virtual command that is build together from what I call base commands (AUTO, HOME, MANUAL, START, STOP). When you send the command EOD, the status/mode returned is HOME and the status/mode returned after midnight is AUTO. That's exactly what the description of the EOD command says:

Return home and quit your work (HOME command). At the next day (when midnight is passed) start over with the timer settings applied (AUTO command).

The separation of the status/mode and the actual commands is more or less required in my opinion and besides gives us the flexibility to define new virtual commands.

I would like to exclude JOB at this time as openHAB has no way to handle this via UI (could only be used by rules).

Hope it's comprehensible :-)

Cheers Ingo

reyem commented 7 years ago

Thank you for your thoughts. I had similar findings but a different conclusion. The issue with what you call "commands" is, that openhab does not have a good way sending them from the UI. All the elements have a state. ON / OFF, PLAY / PAUSE .... Although there are triggers which would not have a state, they cannot be triggered from the UI yet.

I left the Mode to behave as defined in the Robonect API. The mode can be MANUAL, AUTO, HOME when retrieving them as status info. But when using them as commands, also EOD and JOB can be sent. The binding will with the next status update just reset the drop down shown in the UI with the "regular" state the mower was set in.

JOB I also implemented. The binding has channels for the job settings. So even from the UI one can set the settings and then set the Mode to JOB which will start the JOB. This is one of the things which I personally really want to have in the binding, as it allows to define the Timer from openhab. I plan to use the Calendar binding for starting jobs and disable the timer on the robot. This allows me to consider also other sensor values which are already in the system. For example wait with mowing until its not raining anymore. Further I can simply remove calendar entries if I don't want the mower to start on a certain day...

I hope you can live with the String Mode strings ;-) If you do not object, I would close this issue...

itn3rd77 commented 7 years ago

I am fine with what ever you come up as I am sure it will fit me needs. Closing this issue.

Thanks for your time and the great job you are doing!

Ingo