TelluIoT / ThingML

The ThingML modelling language
https://github.com/TelluIoT/ThingML
Apache License 2.0
101 stars 32 forks source link

Multiple syntax for Ports #188

Open brice-morin opened 7 years ago

brice-morin commented 7 years ago

It seems ports can have multiple syntax when it comes to listing the messages to be sent and to be received. For example:

required port p {
    sends a, b, c
    receives x, y, z
}

//or

required port p2 {
    sends a
    sends b
    sends c
    receives x
    receives y
    receives z
}

The rule in XText quite obviously allows for both:

RequiredPort returns RequiredPort:
    (optional?='optional')? 'required' 'port' name=ID ( annotations+=PlatformAnnotation )* '{'
        (('sends' sends+=[Message|ID] ( "," sends+=[Message|ID])* ) | 
        ('receives' receives+=[Message|ID] ( "," receives+=[Message|ID])* ))*
    '}';

Do we really need to provide two alternative syntax for listing messages in ports. I much prefer the first one...

The funny thing is that the new model flattener (or actually the serialization) seems to use one or the other syntax, and I did not really find a way in the formatter to force using just one syntax, yielding funny-looking thingml files.... (the model is anyway the same in the end)

@ffleurey @jakhog what do you think?

ffleurey commented 7 years ago

This a "well known" feature for me. I often use it to group messages which are related. It dates back to the very origins of ThingML. Should we keep it? That is another question :-) I vote for the first too if we keep only one.

brice-morin commented 7 years ago

Or a solution can be to have this notion of grouped messages in the metamodel, so that we do not loose that info when we load and then save the model. So as save(load(myfile.thingml)) is idempotent :-)

brice-morin commented 7 years ago

Though a port is the way to group message... Maybe indicating we need several ports if we feel the need to have groups...

ffleurey commented 7 years ago

Yes.... for the sake of idempotence we should probably just get rid of it :-)

jakhog commented 7 years ago

Well, I have no strong opinions on the matter, but if we choose one, I guess we all agree that it should be the first option to keep.

The grouping argument makes sense, but in my mind, if we want to use different ports or different receives/sends for grouping of messages with similar meaning, is more a question if we want the ports to represent the meaning or actual connections between components.

Continuing with your fancy word - idempotence - what we should pursue is probably equality for load(save(model)) - which is the normal contract for serializing. Making the resulting strings equal isn't really that useful? We just want it to look pretty :)

For the specific problem at hand: I think there's a way to solve it by overriding the IValueSerializer.serializeUnassignedValue method. It's listed as a problem here

brice-morin commented 7 years ago

Yes, the real issue is indeed the formatter, which should prettify the code. Right now, I think the port are uglified by default.