eclipse / vorto

Vorto Project
www.eclipse.org/vorto
Eclipse Public License 2.0
225 stars 105 forks source link

Overriding properties inline #823

Closed Ebolon closed 6 years ago

Ebolon commented 6 years ago

Like discussed in #822 it should be possible to override some attributes inline in a functionblock/entity.

Here some examples: Example1:

namespace com.ipso.smartobjects

functionblock Distance {

   status {
       mandatory sensor_value as float
       optional sensor_units as string 
       ...    
   }
}

Information model using the IPSO model and overriding it:

using com.ipso.smartobjects.Distance;0.0.1

infomodel MyDistance100MeterDevice {

   functionblocks {
       distance as Distance with {
            mandatory sensor_value as float <max:100>
       }
   }
}

Example2:

entity EntityA {
    mandatory valueA as int
    optional valueB as int
}
using de.vorto.EntityA;0.0.1

functionblock FunctionblockA {
   status {
       mandatory entityA as EntityA
       optional unit as string 
   }
}
using de.vorto.FunctionblockA;0.0.1

infomodel DeviceA {
   functionblocks {
       distance as FunctionblockA with {
            mandatory entityA as EntityA with {
                valueA as int <MAX 100>
            }
       }
   }
}
aedelmann commented 6 years ago

I like that change. I think that would make the DSL very powerful because information models would have a way to specialize abstract and re-usable functionblocks in an inline way to further refine the model. Would you dare to extend the Informatioin Model Definition DSL to make this change :) ?

thjaeckle commented 6 years ago

Do I understand correctly that by this I can no longer rely on that a functionblock always contains a well defined (and versioned) contract but that it depends on the context? And even worse I cannot rely on the defined entities because they may also be changed by the context (infomodel).

What is the point of a functionblock then if I cannot rely that it is the same when used in different infomodels?

geglock commented 6 years ago

I fully agree with the doubts brought up by @thjaeckle and would rather propose to enable these kinds of adaptions using inheritance between Functionblocks, so that a Functionblock can be inherited from another and then apply some "overrides". Thus the second Functionblock is still fully reference-able, named, versioned and both variants can be used in different information models.

Ebolon commented 6 years ago

Do I understand correctly that by this I can no longer rely on that a functionblock always contains a well defined (and versioned) contract but that it depends on the context?

It depends on your information model (IM) which is the base for all Vorto models. The Vorto framework currently only uses the generated Ecore classes as they are. This should change in the future to enable the inheritance feature of a functionblock (see #796). As a user of the Vorto framework you need not to care about any extensions of a functionblock. In the future there will be a extended Ecore model which can be used by the generators.

What is the point of a functionblock then if I cannot rely that it is the same when used in different infomodels?

The benefit is that you can change or extend specific parts like the anonymous class concept known from programming languages. The point of a functionblock stays the same you have a common base for development.

Maybe we can align how you use the Vorto DSL. Have you a scenario in mind where this language feature could be a problem?

thjaeckle commented 6 years ago

Yes, I have scenarios in mind where this is a problem :)

Coming from the "digital twin" (Eclipse Ditto) side where the information models and functionblocks come to live with concrete instances assume you have:

With the discussed overriding properties inline the "cargo-container:2.0.0" could now choose that the longitude and latitude of the "locatable:1.0.0" functionblock are no longer doubles but Strings instead.

So an instance of a car would look in JSON like:

{
  ...
  "locatable": {
    "longitude": -73.989308,
    "latitude": 40.712775
  }
  ...
}

An instance of a cargo-container however would look in JSON like:

{
  ...
  "locatable": {
    "longitude": "-73.989308 long",
    "latitude": "40.712775 lat"
  }
  ...
}

So both infomodels contain the same functionblock in the same version but they do not share the same structure.

In Ditto we want to provide search queries like:

Do you see that this no longer makes any sense if the contract of a functionblock is no contract any more but totally dynamic and dependent on the context (infomodel)?

I am not interested in the Vorto DSL - I am "only" interested in using functionblocks with a given namespace, name and version as contract that I can rely on that the structure is always the same, no matter in which infomodels it may be used.

aedelmann commented 6 years ago

Hi @thjaeckle @geglock I understand your concern and agree that as in your example, that must not be possible to override types and thus structure of the function block (As I mentioned in the current Pull Request) It should only be possible to specialise it by adding constraints to it. Otherwise, I agree with you, it would "break" the basic contract as defined in the function block.

Ebolon commented 6 years ago

See your point! Like in OOP-languages it should not possible to change the type of a property. Only the with, constraints, and description block can be altered. Additionally new properties can be introduced. The same goes for the extends feature. I add appropriate validation to the IDE in the future.

From my point of view your feature would not be affected. Are you agree?

Currently a property is described as:

Property:
    (presence = Presence)? (multiplicity ?= 'multiple')? name = ID 'as' type = PropertyType
    ('with' '{' propertyAttributes+=PropertyAttribute (',' propertyAttributes+=PropertyAttribute)* '}')?
    ('<' constraintRule = ConstraintRule '>')?
    (description=STRING)?
;
DSL keyword description overridable
presence is a property mandatory/optional no 
multiplicity multiple of... no
name name of property no 
type type of property no
with  additional property attributes yes
constraintRule rules like MIN, MAX for a property yes
description description of a property yes
aedelmann commented 6 years ago

@Ebolon @thjaeckle @geglock Just to clarify, a function blocks declares the basic functionality and information models referencing this function block must at least implement this functionality. BUT it should also be possible to further extend the basic definition of the function block by adding properties and/or adding constraints in the context of the information model without the need to define information model specific function block definitions. I give an example where I think this feature is useful:

functionblock Distance {
   status {
       mandatory distance as float
   }
}

infomodel ShortRangeDevice {
   functionblocks {
      distance as Distance with {
           mandatory distance as float <MIN 0, MAX 50> with {measurementUnit: Unit.METER}
      }
   }
}

infomodel LongRangeDevice {
   functionblocks {
      distance as Distance with {
           mandatory distance as float <MIN 0, MAX 100> with {measurementUnit: Unit.METER}
      }
   }
}

// Should not be possible as it breaks the basic type definition
infomodel InvalidDevice {
   functionblocks {
      distance as Distance with {
           mandatory distance as string
      }
   }
}

Having said that and I totally agree to the concerns brought up, that the basic definition must not be broken by the extension.

aedelmann commented 6 years ago

Hi @Ebolon good summary and I would agree to that for properties that exist in the base function block definition. But it should still be possible to add more properties to it in the context of the information model.

thjaeckle commented 6 years ago

So you can change the constraints which are defined in a functionblock to other constraints?

If the Distance FB defines the following:

mandatory distance as float <MIN 0, MAX 50> with {measurementUnit: Unit.METER}

May the informationmodel define:

infomodel LongRangeDevice {
   functionblocks {
      distance as Distance with {
           mandatory distance as float <MIN 0, MAX 100> with {measurementUnit: Unit.METER}
      }
   }
}

Or may it only restrict existing constraints further? What about regular expressions then? Seems like almost impossible to correctly do that ..

aedelmann commented 6 years ago

@thjaeckle IMO, it must not break the existing function block definition, even if it defines certain constraints, like regex or measurement units. But it would be possible to add them, if the basic definition does not define them, that includes contraintRules, with and description.

thjaeckle commented 6 years ago

Ok, great. Then we agree :) I however don't see the benefit in it then - only adds complexity to the DSL which won't be needed that often. KISS & YAGNI principles apply ;)

aedelmann commented 6 years ago

@thjaeckle Well, I would foresee that this feature is used for all devices that would wanna add their device specific properties without the need to create device - specific function blocks. Take the IPSO models as an example, like Temperature. The Temperature FB is so generic and merely defines the value and unit. But a device that wants to be IPSO compliant may now use this function block and further specialise it by adding a min and max for the temperature value as well as a regular expression for the unit. I think it would be good if Eclipse Ditto becomes also aware of Information Models in addition to function blocks, a thing is described by. That way you can also have a way to validate information model specific properties.

thjaeckle commented 6 years ago

If only the infomodels contain "the whole truth" about the actual structure, Ditto should probably only look at them and forget about the functionblocks. So a Ditto feature could be the instance of 1..n infomodels.

geglock commented 6 years ago

I still don't see a benefit to allow these overrides to be done "inline" while linking a function block into an information model. It would be much more transparent to define a derived function block "MyTemperature" that refines the general IPSO "Temperature". Then all the versioning, re-use, management mechanisms would still be in-place and at the same time the DSL/meta model would stay simple but still providing all those override capabilities.

aedelmann commented 6 years ago

@geglock Yeah, of course you could do that but in this case, that function block becomes potentially re-usable by other information models but no other information model would ever re-use it because it is too specific. I mean, who would re-use a "MyTemperature" function block which defines e.g. a max value of 50 degrees ? In fact, this would result in thousands of different Temperature function block flavours (e.g My60DegreesTemperature, ...) snow-balling to be quite messy and confusing for people which one to choose to describe his/her device. In the end they would define their own again. IMO device specific properties should be defined within the information model itself whereas device generic device functionality as a limited set of re-usable function blocks.

e-grigorov commented 6 years ago

My understanding on this topic is that there are:

If this feature is integrated, no such thing like "description" will exist any more. The application has to merge the info model with the function block to find the function block complete description. To me, it'll break the main goal of Vorto: clear models.

To compare with OOP, the subsclass should not change the contract from the base class - Liskov substitution principle

geglock commented 6 years ago

For me this topic looks like "anonymous inner classes" in Java - despite the discussion about breaking or not breaking the contracts.

But then major difference is that in Java these anonymous classes are only/exclusively used by the implementation in the same area where the anonymous class was defined. But in Vorto we are doing something completely different: we are "exporting" these anonymous definitions the externals. Even more, Vorto is mainly there for exporting definitions. And so anonymous definitions would contradict the whole idea.

Regarding the comment about "MyTemperature". Of course this would effect that there are a plenty of different function block flavors. But therefore we have namespaces and - hopefully in the future - also different repositories (e.g. private repositories).

So I would vote for reverting this "inline" approach but refactor it to an "extend" functionality between Functionblocks.

aedelmann commented 6 years ago

If this feature is integrated, no such thing like "description" will exist any more. The application has to merge the info model with the function block to find the function block complete description. To me, it'll break the main goal of Vorto: clear models.

The descriptions in function blocks and datatypes remain so that will not break. what we additionally adding here with this inline "extend" is that it provides a way to define information model specific properties and constraints , in addition to the one's that the function block defines and the information model adheres to. Currently there is no convenient way in Vorto to define device (information model) specific properties. But where would this information be otherwise defined in your opinion @e-grigorov ? IMO, it cannot be Function blocks, because they represent abstract definitions independent of the device. I think it is still ensure that Vorto has clear models: Function blocks model for abstract definition of device functionality, and information models defining concrete device specific functionality by re-using abstract function blocks + device specific properties.

@geglock : We do support "extend" between function blocks as of today. The question is should this be used to also express device specific functionality as well compared to putting this info into information models themselves. This extension cannot be compared to anonymous classes as they both fulfil different purposes. The purpose here is to express a way to further "annotate" function blocks with device specific information.

e-grigorov commented 6 years ago

Currently there is no convenient way in Vorto to define device (information model) specific properties. But where would this information be otherwise defined in your opinion @e-grigorov ? IMO, it cannot be Function blocks, because they represent abstract definitions independent of the device.

In general, the device itself can be a function block. Usually, the device representation contains properties like:

This device can be defined as:

functionblock DeviceInfo {
  status {
    ...
    model as string
    firmwareVendor as string
    ...
  }
}

I have the feeling that you are talking about another use case here. It seems that you would like to specify device specific property values. For example: model - XYZ, firmwareVendor - companyXYZ:

infomodel {
  functionblocks {
    deviceInfo as DeviceInfo {
      model=XYZ
      firmwareVendor=companyXYZ
    }
  }
}

@aedelmann, is it correct?

Ebolon commented 6 years ago

Hey @thjaeckle, @geglock, @e-grigorov,

thank you very much for your input! I had a call with @aedelmann and we agree that for now the benefit is to small (syntactic sugar) to introduce new complexity to the model. I will focus now on the extends feature (see #796) the language already provides and implement the validation and support for the generators.