TelluIoT / ThingML

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

Composite states properties are initialised only once #282

Closed fungiboletus closed 5 years ago

fungiboletus commented 5 years ago

Considering the following composite state :

composite state Lapin init Increment {
  property numberOfLapins: UInt8 = 0

  state Increment {
    on entry do
      numberOfLapins = numbersOflapins + 1
      print numberOfLapins
      timer!timer_start(42, 1000)
    end

    transition -> Increment event m:timer?timer_timeout guard m.id == 42
  }

  transition -> Canard event internalPort?goToCanard
}

I did expect numberOfLapins to be initialised to 0 everytime my state machine enters the composite state Lapin. But it does not. When I look at the generated code in JavaScript, the property is actually global and initialised only once per instance like every other property.

I guess this is normal considering the current implementation, but is it the correct behaviour ?

The following code works as I expect it to.

composite state Lapin init Increment {
  property numberOfLapins: UInt8 = 0

  on entry do
    numberOfLapins = 0
  end

  state Increment {
    on entry do
      numberOfLapins = numbersOflapins + 1
      print numberOfLapins
      timer!timer_start(42, 1000)
    end

    transition -> Increment event m:timer?timer_timeout guard m.id == 42
  }

  transition -> Canard event internalPort?goToCanard
}
brice-morin commented 5 years ago

As #203 points out already, it is probably more correct to re-initialize them when we re-enter the state (except if the composite state keep history). We could still keep those properties global in the generated code (as I suspect it makes it simpler in the compiler), but we should be able to insert the re-initialization code systematically.

As you found the official work-around, I assume the fix is not completely urgent :-) But we should fix it

brice-morin commented 5 years ago

Also related to #197 The general issue of re-initializing properties gets more complex with keeps history. But it seems that discussion tended towards a shallow history. But we did not conclude anything on whether or not atomic states should be allowed to keep history or not. @ffleurey ???

Anyway, in your case (without history), properties should indeed be reset.

But I would rather fix the general issue, rather than just the special case. IMHO, if we go for shallow history AND allow atomic state to have history (AND reset by default is no history), I think we can then have a quite flexible strategy. @ffleurey ???

brice-morin commented 5 years ago

Your issue should be fixed (for all compilers).

Note that the commit just fix your issue, i.e. reset properties of composite states that do not have history.