Skip to content
This repository has been archived by the owner on May 7, 2020. It is now read-only.

Configurable ItemChannelLinks #4323

Merged
merged 1 commit into from
Sep 28, 2017

Conversation

sjsf
Copy link
Contributor

@sjsf sjsf commented Sep 22, 2017

This allows adding some configuration properties onto links, e.g. though

  Switch someItem {
      channel="a:b:c:channel" [
          foo="bar",
          answer=42,
          always=true
      ]
  }

or the REST API.

As a consequence, ItemChannelLinks now also can be updated through the REST API.

❗️ This is API breaking, it adds a Configuration argument to BindingConfigReader.processBindingConfiguration(...). I wouldn't expect many implementations out there - apart from the OH1 compatibility layer. Let me know any concerns, than I will think of something different - but I didn't want to clutter the API without a need.

fixes #583
Signed-off-by: Simon Kaufmann simon.kfm@googlemail.com

Copy link
Contributor

@triller-telekom triller-telekom left a comment

Choose a reason for hiding this comment

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

Two small comments from my side, please see them inline in the code.

return Response.status(Status.BAD_REQUEST).build();
}
link = new ItemChannelLink(itemName, new ChannelUID(channelUid), new Configuration(bean.configuration));

Copy link
Contributor

Choose a reason for hiding this comment

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

Unnecessary newline

;

NUMBER returns ecore::EBigDecimal:
ID ('.' ID )?
Copy link
Contributor

Choose a reason for hiding this comment

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

A number can contain any char from ID, so also a-z, A-Z, _?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Stolen from Thing.xtext. Why should it be different?

Copy link
Contributor

Choose a reason for hiding this comment

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

@triller-telekom Think about numbers like -4.52E+6 and you will understand why it is that way :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

@kaikreuzer Thank you for this explanation, but I still don't understand why you allow different characters than e or E and _ for a number. Or is it just out of convenience reasons that you do not want to define a slightly different ID terminal? :)

This allows adding some configuration properties onto links, e.g. though

  Switch someItem {
      channel="a:b:c:channel" [
          foo="bar",
          answer=42,
          always=true
      ]
  }

or the REST API.

As a consequence, ItemChannelLinks now also can be updated through the REST API.

fixes eclipse-archived#583
Signed-off-by: Simon Kaufmann <simon.kfm@googlemail.com>
@sjsf sjsf force-pushed the configurableLinks branch from d2deb3e to 455b687 Compare September 22, 2017 15:17
Copy link
Contributor

@maggu2810 maggu2810 left a comment

Choose a reason for hiding this comment

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

I didn't have a look at the DSL items grammar, but the remaining part looks good to me.

@kaikreuzer kaikreuzer merged commit f4247ba into eclipse-archived:master Sep 28, 2017
@kaikreuzer
Copy link
Contributor

@SJKA A question that I just stumbled over:

As you know, the channel property accepts a list of channels e.g.

channel="a:b:c:channel, d:e.f:channel, g:h:i:channel"

How does your chosen syntax for the configurations fit to this?

@sjsf
Copy link
Contributor Author

sjsf commented Oct 3, 2017

The same configuration obviously gets applied to all the links, i.e.

      channel="a:b:c:channel, d:e:f:channel, g:h:i:channel" [
          foo="bar",
          answer=42,
          always=true
      ]

However, as far as I can see it is allowed to use the same binding configuration multiple times, i.e. if there should be different link configurations, then you'd simply split the one string up and make it three different ones like this:

      channel="a:b:c:channel" [
          foo="bar",
          answer=42,
          always=true
      ],
      channel="d:e:f:channel" [
          foo="bar",
          answer=43,
          always=true
      ],
      channel="g:h:i:channel" [
          foo="bar",
          answer=44,
          always=true
      ]

@kaikreuzer
Copy link
Contributor

as far as I can see it is allowed to use the same binding configuration multiple times

Ok, thanks, that would be a solution. Do you just "believe" that it should work or did you check it?

sjsf pushed a commit to sjsf/smarthome that referenced this pull request Oct 5, 2017
see eclipse-archived#4323
Signed-off-by: Simon Kaufmann <simon.kfm@googlemail.com>
@sjsf
Copy link
Contributor Author

sjsf commented Oct 5, 2017

I'm a strong believer in things I have tried out!
I created #4378 in order to make you become a believer too 😉

@kaikreuzer
Copy link
Contributor

Thanks :-)

kaikreuzer pushed a commit that referenced this pull request Oct 5, 2017
see #4323
Signed-off-by: Simon Kaufmann <simon.kfm@googlemail.com>
@kaikreuzer kaikreuzer added this to the 0.9.0 milestone Nov 30, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for properties on ItemChannelLinks
5 participants