UoY-RoboStar / robosim-textual

This repository contains the plugins for the RoboSim textual editor.
Eclipse Public License 2.0
0 stars 0 forks source link

Not possible to connect two ControllerRefs in a SimModule (EValidator issue) #1

Open pefribeiro opened 4 years ago

pefribeiro commented 4 years ago

Currently the implemented validation rules do not cover the case of connections between controllers, so an error is raised by EValidator.

Example:

module Mod {

    cycleDef 1

    rref rp=M::RP
    cref c1=SimC1
    cref c2=SimC2   

    connection c1 on c2c        to c2 on c2c 
}

The validation fails because of connection c1 on c2c to c2 on c2c.

I looked at the code in RoboSimValidator.xtend and I am wondering what is the purpose of checkConnectionDirection. Isn't this validation already covered by the RoboChart rules in RoboChartValidator.xtend @madielfilho? I think we should avoid duplicating any validation that is already part of RoboChart unless it is about elements specific to RoboSim.

I am happy to remove the duplicated code if you can confirm this please @madielfilho. I see that there are other duplicated rules (eg. checkClockExpWellTyped and variableInitWellTyped. Can't these be removed?

pefribeiro commented 4 years ago

I realise there is a comment on line 337:

// The typeFor is an injected method and is required in RoboSim for condition with the Feedback Expression
// If this method is removed, or its superclass method is called, the typeFor won't use RoboSim's injected typeFor.

I guess, this implementation can be improved by defining a method in the parent class, and overriding it, similarly to what @alvarohm has done for the revised TimedGenerator.xtend implementation. Is that possible for the injected RoboSimTypeProvider?

madielfilho commented 4 years ago

Hi Pedro,

The validation fails because of connection c1 on c2c to c2 on c2c.

Okay. I will check it out.

I looked at the code in RoboSimValidator.xtend

https://github.com/robo-star/circus.robocalc.robosim.textual.parent/blob/a2f56b1ddf42e838a331db431c1b6644a9a6f135/circus.robocalc.robosim.textual/src/circus/robocalc/robosim/textual/validation/RoboSimValidator.xtend#L400 and I am wondering what is the purpose of checkConnectionDirection. Isn't this validation already covered by the RoboChart rules in RoboChartValidator.xtend @madielfilho https://github.com/madielfilho?

I think so. The method checkConnectionDirection was initially implemented to check all connections, but, for SimModule and SimControllers, I think such validations are already covered by the RoboChart rules. In RoboSim, for SimMachineDef, we also need to consider the events (and the interfaces) that can be declared directly in the input/output context.

I think we should avoid duplicating any validation that is already part of

RoboChart unless it is about elements specific to RoboSim.

Yes, absolutely.

I am happy to remove the duplicated code if you can confirm this please

@madielfilho https://github.com/madielfilho. I see that there are other duplicated rules (eg. checkClockExpWellTyped https://github.com/robo-star/circus.robocalc.robosim.textual.parent/blob/a2f56b1ddf42e838a331db431c1b6644a9a6f135/circus.robocalc.robosim.textual/src/circus/robocalc/robosim/textual/validation/RoboSimValidator.xtend#L428 and variableInitWellTyped https://github.com/robo-star/circus.robocalc.robosim.textual.parent/blob/a2f56b1ddf42e838a331db431c1b6644a9a6f135/circus.robocalc.robosim.textual/src/circus/robocalc/robosim/textual/validation/RoboSimValidator.xtend#L524 .

Okay. I also think that these validations are already covered by the RoboChart rules.

Em ter., 28 de jan. de 2020 às 11:03, Pedro Ribeiro < notifications@github.com> escreveu:

Currently the implemented validation rules do not cover the case of connections between controllers, so an error is raised by EValidator.

Example:

module Mod {

cycleDef 1

rref rp=M::RP cref c1=SimC1 cref c2=SimC2

connection c1 on c2c to c2 on c2c }

The validation fails because of connection c1 on c2c to c2 on c2c.

I looked at the code in RoboSimValidator.xtend https://github.com/robo-star/circus.robocalc.robosim.textual.parent/blob/a2f56b1ddf42e838a331db431c1b6644a9a6f135/circus.robocalc.robosim.textual/src/circus/robocalc/robosim/textual/validation/RoboSimValidator.xtend#L400 and I am wondering what is the purpose of checkConnectionDirection. Isn't this validation already covered by the RoboChart rules in RoboChartValidator.xtend @madielfilho https://github.com/madielfilho? I think we should avoid duplicating any validation that is already part of RoboChart unless it is about elements specific to RoboSim.

I am happy to remove the duplicated code if you can confirm this please @madielfilho https://github.com/madielfilho. I see that there are other duplicated rules (eg. checkClockExpWellTyped https://github.com/robo-star/circus.robocalc.robosim.textual.parent/blob/a2f56b1ddf42e838a331db431c1b6644a9a6f135/circus.robocalc.robosim.textual/src/circus/robocalc/robosim/textual/validation/RoboSimValidator.xtend#L428 and variableInitWellTyped https://github.com/robo-star/circus.robocalc.robosim.textual.parent/blob/a2f56b1ddf42e838a331db431c1b6644a9a6f135/circus.robocalc.robosim.textual/src/circus/robocalc/robosim/textual/validation/RoboSimValidator.xtend#L524. Can't

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/robo-star/circus.robocalc.robosim.textual.parent/issues/1?email_source=notifications&email_token=AMFBJWZQW7KGPUV2DNFJVA3RAA3KHA5CNFSM4KMS6LPKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4IJHMWFA, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMFBJWYJEMI6KPJIPGINZNLRAA3KHANCNFSM4KMS6LPA .

pefribeiro commented 4 years ago

Thanks @madielfilho for clarifying these aspects.

I think so. The method checkConnectionDirection was initially implemented to check all connections, but, for SimModule and SimControllers, I think such validations are already covered by the RoboChart rules. In RoboSim, for SimMachineDef, we also need to consider the events (and the interfaces) that can be declared directly in the input/output context.

Ok, this is related to issue #3, which is just filled in.