eclipse-archived / smarthome

Eclipse SmartHome™ project
https://www.eclipse.org/smarthome/
Eclipse Public License 2.0
862 stars 783 forks source link

[REST Api] Inconsistent response of REST-Api for items with mapped values #2788

Closed adb76 closed 7 years ago

adb76 commented 7 years ago

I'm currently using openHAB 2 build #692 with the following items / map Definition (as also described here: https://community.openhab.org/t/mapping-of-item-values-not-correctly-shown-in-habpanel/19518)

Contact Wohnzimmer_Fenster1_OpenClosed  "Wohnzimmerfenster 1 [MAP(de.map):%s]" (Wohnzimmer, Wohnung_Fenster) { channel="max:shuttercontact:NEQ0897054:LEQ0791036:contact_state" }

Contact Wohnzimmer_Fenster2_OpenClosed  "Wohnzimmerfenster 2 [MAP(de.map):%s]" (Wohnzimmer, Wohnung_Fenster) { channel="max:shuttercontact:NEQ0897054:LEQ0790888:contact_state" }

de.map:

CLOSED=zu
OPEN=offen

When I make a REST-Api call http://homenuc:8080/rest/items/Wohnzimmer_Fenster1_OpenClosed/state

it returns always CLOSED respectively OPEN

When you call the item with http://homenuc:8080/rest/items/Wohnzimmer_Fenster1_OpenClosed

returns the mapped value "zu" and "offen" beside the key "state":

{
  "link": "http://homenuc:8080/rest/items/Wohnzimmer_Fenster1_OpenClosed",
  "state": "zu",
  "stateDescription": {
    "pattern": "",
    "readOnly": false,
    "options": []
  },
  "type": "Contact",
  "name": "Wohnzimmer_Fenster1_OpenClosed",
  "label": "Wohnzimmerfenster 1",
  "tags": [],
  "groupNames": [
    "Wohnzimmer",
    "Wohnung_Fenster"
  ]
}

The same also when calling it via a "sitemaps" call like:

http://homenuc:8080/rest/sitemaps/zuHause?jsoncallback=callback

"widgets": [
                      {
                        "widgetId": "Wohnzimmer_0",
                        "type": "Text",
                        "label": "Wohnzimmerfenster 1 [zu]",
                        "icon": "contact",
                        "mappings": [],
                        "item": {
                          "link": "http://homenuc:8080/rest/items/Wohnzimmer_Fenster1_OpenClosed",
                          "state": "zu",
                          "stateDescription": {
                            "pattern": "",
                            "readOnly": false,
                            "options": []
                          },
                          "type": "Contact",
                          "name": "Wohnzimmer_Fenster1_OpenClosed",
                          "label": "Wohnzimmerfenster 1",
                          "tags": [],
                          "groupNames": [
                            "Wohnzimmer",
                            "Wohnung_Fenster"
                          ]
                        },
                        "widgets": []

On the "Event channel" it is also always "unmapped". This leads for example in HABpanel to the problem that the values are "jumping" between their mapped and unmapped representation dependent of the way the last call was made.

I would have had expected, that the values are always send in der mapped representations via the REST Api.

Kind regards,

André

sjsf commented 7 years ago

This inconsistency indeed is weird... Thanks for reporting this!

Before I break bazillions of SmartHome solutions: Does anybody (@kaikreuzer, @cdjackson?) know any good reason for the states NOT being transformed at those two places (i.e. rest/items/.../state and in the Events)?

kaikreuzer commented 7 years ago

Before I break bazillions of SmartHome solutions

You are standing a good chance to do so ;-)

any good reason for the states NOT being transformed at those two places (i.e. rest/items/.../state and in the Events)?

They simply provide the REAL item state, i.e. the full truth. Note that item states are for "headless" use, i.e. not UI related at all.

When defining an item like

Contact Wohnzimmer_Fenster1_OpenClosed  "Wohnzimmerfenster 1 [MAP(de.map):%s]" 

you have to note that the part in quotes specifies a "default label" to use, if no other label is specified in a UI. So while there is one single state of the item, it might have many different visual representations depending on its usage. That's why the sitemap endpoint provides the mapped state (but note that you could have specified the mapping in the sitemap as well and this could be a different one than the default given in the item file).

kaikreuzer commented 7 years ago

FTR, actually, I think the correct fix would be to NOT map it in the sitemap endpoint either. See this discussion: https://github.com/openhab/openhab.android/issues/193#issuecomment-226801212

Would that make sense?

adb76 commented 7 years ago

Just for the novice openHAB user:

They simply provide the REAL item state, i.e. the full truth. Note that item states are for "headless" use, i.e. not UI related at all.

Wouldn't it then be better also not to map it in the call: http://homenuc:8080/rest/items/Wohnzimmer_Fenster1_OpenClosed

EDIT: Further question: If we then always get the unmapped value, this would mean that each UI has to implement their own mapping - right? Or would it be feasible to provide both values the original raw value and the mapped value from the items Definition?

kaikreuzer commented 7 years ago

Yes, I agree.

maggu2810 commented 7 years ago

Wouldn't it then be better also not to map it in the call: http://homenuc:8080/rest/items/Wohnzimmer_Fenster1_OpenClosed

Yes, I agree.

Me, too.

sjsf commented 7 years ago

So the remaining question is (which also came up in the android client related discussion): Do we need an additional field in the EnrichedItemDTO containing the mapped value? Keep in mind, we have the mapped value as part of the label already. Except in the ItemStateChangedEvent...

adb76 commented 7 years ago

Do we need an additional field in the EnrichedItemDTO containing the mapped value?

From my (maybe to simple) understanding I would assume "yes". The mapping is often used as a translation of the language or a translation to have a better understanding of the meaning of the value. If the REST-Api omits that, every UI which uses the REST services directly would have to implement such a translated view. Wouldn't it then be better to define a translation at one place and provide that as an alternative view to all UIs via the REST Api?

But maybe my understanding is totally wrong here... ;-)

lolodomo commented 7 years ago

Any chance to make progress on this issue, I would be happy if HABdroid could display the right icon.

metbril commented 7 years ago

I am using mapped states to localize and or prettify my values. If I need dynamic icons as well then I (and others with me) would need to create custom icons for each mapping. This would be tedious if impossible at least, since there is no way to know what characters are used in a mapping (Unicode emoticons?)

Therefor I think it would be best to follow 1.x behavior and ALWAYS use the raw state for the dynamic icon in any UI.

Additionally it is good to notice that a state that is mapped in an item label definition still needs to be addressed by its raw state in a rule!

LKuech commented 7 years ago

This bug is very confusing when using HabPanel to show important item states like contacts (which are defined with a transform). e.g. using a dummy widget for the contacts: If the page is refreshed the shown text value is transformed and is reflection the real state, but in the same moment the icon is wrong as the icon provider does not understand the transformed value (in my case "Offen" for OPEN and "Zu" for CLOSED). As soon as the Item is receiving an update (while the page is still shown and not manually refreshed), the text state is changing to a non-transformed state and the icon is working perfectly.

The only way to keep it user friendly right now seems to be to remove the transform from the item definition and stay with the raw states ... which means English values are on the dashboard...

metbril commented 7 years ago

@LKuech to me this sounds as another use case to use the raw state for determining dynamic items and use the mapped state for display purposes in any UI.

lolodomo commented 7 years ago

@LKuech : it is exactly the same thing for HABdroid. A very annoying bug that probably impacts all non english users.

maggu2810 commented 7 years ago

WRT to

Wouldn't it then be better also not to map it in the call: http://homenuc:8080/rest/items/Wohnzimmer_Fenster1_OpenClosed

I added the REST and bug label

lolodomo commented 7 years ago

Ok, I think it is time to make progress on this issue.

Here is the proposal following the full discussion:

I can apply this change in ESH. HABdroid should then be fixed automatically (except one case that was mentioned in the HABdroid issue that would require a fix to use the new field - setpoint).

@kaikreuzer @maggu2810 : do I have a GO ?

maggu2810 commented 7 years ago

Okay for me

kaikreuzer commented 7 years ago

For me as well, thanks!

lolodomo commented 7 years ago

I just implemented the change and it works well (including in HABdroid).

Just one last question: in most cases, "state" and "transformedState" have the same value. Do you want I set a null string for "transformedState" in case there is no transformation ? It will just reduce the size of messages in particular when your state is a data URI (image).

I will propose a PR as soon as my PR relative to image rendering is merged (a change in a common file is required).

maggu2810 commented 7 years ago

in most cases, "state" and "transformedState" have the same value. Do you want I set a null string for "transformedState" in case there is no transformation ? It will just reduce the size of messages in particular when your state is a data URI (image).

Are we able to remove the transformedState from the REST response if it is equal to state? So the client could assume if transformedState is missing, then there is no transformation or the transformation is equal to the state itself.

lolodomo commented 7 years ago

@maggu2810 : removing the transformedState from the REST response would probably mean defining two different DTO objects :( That's why I am proposing to keep the transformedState field in all cases but set it to null when there is no transformation.

kaikreuzer commented 7 years ago

set it to null when there is no transformation.

Would be ok for me - although it requires the clients to implement some more logic on their end.

lolodomo commented 7 years ago

I will update my PR accordingly.

lolodomo commented 7 years ago

Is there existing documentation I should update to present the new field in REST API response?

kaikreuzer commented 7 years ago

Hm, no, unfortunately, there is no documentation for the sitemap REST API.

lolodomo commented 7 years ago

I updated the PR. Finally, the transformedState is not present in the REST API response when there is no transformation requested. To be honest, I am not absolutely sure if it is not present or just not displayed by the REST API UI. Using the REST API UI, I only see the transformedState field when a MAP is used in the item label.