Fixed issues #30, #32, #33, #34 and #36.

[christoff-buerger]

Fix for issue #30: Increased sampling times of the synchronous variants of the cascade-controlled drive example by factor 10. A higher sampling is required since the system is unstable with its current sampling (this can be best seen by increasing the simulation time from 4s to 40s).

Fix for issue #32: Event-clock-signal propagation of engine-throttle control example is now done via visible diagram-layer components. For an overview cf. https://github.com/christoff-buerger/Modelica_Synchronous/blob/master/Modelica_Synchronous/Resources/Images/Examples/EngineThrottleControl_Model.png.

Fix for issue #33: The rotational clock of the engine-throttle control example is now done via visible diagram-layer components, helping to improve understanding of the clock's self-dependency on its last tick for deciding future ticks. For an overview cf. https://github.com/christoff-buerger/Modelica_Synchronous/blob/master/Modelica_Synchronous/Resources/Images/Examples/RotationalClock_Model.PNG.

Fix for issue #34: The rotational clock of the engine-throttle control example is now generalized to support any rotational interval the input angle must be changed to trigger the next clock tick. To that end, the clock is plit into two: a fixed rotational clock where the trigger interval is a parameter and a rotational clock where the trigger interval is an input that can change throughout time (for example depending on another clock). Both rotational clocks are be provided within a Rotational sub-package of ClockSignals.Clocks.

Fix for issue #36: Rotational clocks now provide two further values clocked with the provided output clock signal. The first is an integer encoding the rotation direction and a second is a boolean telling whether the rotation direction changed since the last tick or not. For a simple example consider the test in Examples.Elementary.ClockSignals.RotationalSample. It is a rotational clock with variable trigger interval and switching rotation-direction. The input rotation is just sinodical, switching direction every half second. The trigger interval is changed with the same pace; every half second it is doubled or halfed respectively. For the generated clocked signals cf. https://github.com/christoff-buerger/Modelica_Synchronous/blob/master/Modelica_Synchronous/Resources/Images/Examples/RotationalSample_Result.png.

The fixes for issue #32 to #34 also comprise an improved documentation explaining more detailed in how far the rotational clock depends on its own sampling, i.e., how the input angle of the last time its event-condition changed from false to true has to be bookkeeped to decide future ticks. The improved documentation is shown in the info-layer of EngineThrottleControl.

[bernhard-thiele]

Thanks for the patch, it looks good to me and I like the refactoring of the clock of the engine-throttle control example. From a technical side, I would merge it. I'm not sure how @MartinOtter wants to handle contributions to this library at the moment. There is no contributor agreement set up, yet, and the library is still Modelica License 2, while the MSL has moved to a 3-Clause BSD License. I guess it should be no problem since you are anyway MA member and probably MA contribution rules apply (so the library can be moved to 3-Clause BSD License later without needing individual contributors consent).

[christoff-buerger]

@bernhard-thiele Thanks a lot for the feedback!

About licensing: Since the library is within the modelica user group, I consider it under the umbrella of the Modelica Association (MA); its license therefore should be the one adopted by the MA. I made another issue for that (issue #35).

About the clock refactoring (following suggestions are done in commits d4cfd326d68c0d213b5896cff7041011335075f8 and 3501ad8e70dfced0fbd426de6da3285864ae6f0d): I got some ideas the weekend for a few future improvements regarding rotational clocks. I would like to add another clock signal output ticking whenever the rotation-direction changed; that signal would be only updated when the interval clock ticks. Further, I would like to make the trigger_interval an input, such that it can change throughout simulation (for example depending on another clock). To that end, I would separate the clock into a FixedRotationalClock and a RotationalClock, with the former reusing the latter by setting its trigger_interval to a constant. These rotational clocks, I would add to a Rotational subpackage within ClockSignals.Clocks.

The synchronous library also could need some refactoring to reduce code duplication and improve diagram- and code-layout, but that I would keep for a later time.

[christoff-buerger]

I am happy with all changes and improvements now; it can be merged.

[bernhard-thiele]

I asked Martin Otter and he is fine that I handle this merge. So I looked at the changes more closely and got a bit confused.

The new feature for ticking a clock when the rotation direction changes will not tick immediately when the rotation direction changes. Instead it only ticks "synchronously" to the rotational interval based clock (actually infinitesimal later), if there was a change of direction within the time window between the last tick of the interval dependent clock and the current tick. I find this a bit hard to grasp for a user and also the mechanics in which this "direction change" tick is created is not easy to understand due to the interplay of clocked events and non-clocked events. Since it is hard to grasp I lean to rather do without that feature. Do you have a use-case in mind for that feature?

[christoff-buerger]

The main reason to provide information about the rotation-direction with the rotational clock actually is that it is not simple to extract that very information w.r.t. the provided sampling. The dependency on the trigger_interval is deliberate; it is that very interval which defines the whole clock. If you are interested in immediate changes of direction within the trigger_interval, you need another, independent sampling introducing an independent clock partition. In other words, it is another rotational sensor you want in that case.

I can see your point though! I think the main confusion here is that the current implementation provides two clock outputs, which are completely separate clocks. I can change that and only provide one clock output and other further rotational information as ordinary values sampled with the provided clock. Thus, the direction_change output would be boolean instead and part of the clock output's clock partition, i.e., sampled with the rotational clock. Further, I could introduce another integer output which is the direction sign; it will be clocked with the provided clock output as well.

The rational to sample provided rotation information outputs with the provided clock is, that the whole rotational clock should be defined in terms of the trigger interval (the rotation observed for the clock to fire). If you need more precise direction changes you have to decrease the trigger interval. If you like to have both, a slowly (i.e., long trigger interval) clocked rotational clock and a fast direction change clock you have to model both separately, as it intuitively makes sense (it cannot be the same clock).

We could also add some clock logic means to Modelica_Synchronous, like an and-clock that has arbitrary many clocked inputs and ticks if all of them ticked, each at least once. Or an or-clock that ticks whenever any of its inputs ticked. There exists no not in that logic though.

[christoff-buerger]

About the use case: to have the direction check only performed after the input moved at least some rotational interval is for example useful to filter out small disturbances and not tick for every little change. It is also useful to avoid small numeric variations due to close around 0 rotation changes. Since the trigger_interval is a variable input value, one can also model feedback loops changing the interval depending on the rotational speed, like reducing it for a slow rotation and increasing it for fast; or vice-versa depending if you like to suppress or increase the rotational-speed impact.

[christoff-buerger]

Commit 137b8f349bf0af5afd2266ca08c0a5269cadb838 incorporates my last above comments.

@bernhard-thiele Could you please review again and check if your concerns are now handled?

2018-07-28: I forgot to update the model-diagram png of the rotational clock linked in the pull request's description and the engine-throttle control example's documentation. I will do that on Monday.

[bernhard-thiele]

Having the signals direction_changed and direction sampled with the rotational clock makes much sense to me. There are however some details that need additional consideration.

1. Using the Modelica.Blocks.Math.IntegerChange block in a clocked partition is problematic since it it uses the operator change(v) which the MLS 3.4 specifies as: "Is expanded into “(v<>pre(v))”. The same restrictions as for the pre()operator apply." The problem is that MLS 3.4, Section 16.8.1 specifies that these event related operatiors may not be part of a "clocked discrete-time" partition. Maybe the MLS should be relaxed, but currently we should rebuild the logic of the IntegerChange block with previous(v) constructs in order to be standard conformant.

2. The value of the direction_changed variable is always true at the first clock tick. There is a parameter which allows the user to decide the initial value for the direction_changed variable, but it doesn't work. It seems good to allow the user to specify the value of the direction_changed variable at the first clock tick and have a logic that defaults to false if the user doesn't specify anything.

[bernhard-thiele]

Regarding your proposal to add and- and or-clocks I'm not sure how you would do that (I think it can't be done). However, I'm very interested if you have an idea how to do it.

[christoff-buerger]

@bernhard-thiele I fixed your point 1 (commit 7483ebff69c7accdd369a528fd1a0fac02be662a): There is now a new Modelica_Synchronous.IntegerSignals.NonPeriodic.IntegerChange block that implements Modelica.Blocks.Math.IntegerChange in terms of previous instead of change. Respective documentation is added; particularly, that the new block might become superfluous if change ever gets well-defined for clocked discrete-time partitions. Note, that at least in Dymola change is working well if used in clocked discrete-time partitions.

I also updated the model-diagram png of the rotational clock linked in the pull request's description and the engine-throttle control example's documentation.

[christoff-buerger]

Regarding and- and or-clocks: I think that is rather straight forward, by using some kind of flip-flop logic for tracking if some input-clock ticked and respective updating when the output ticks. But I would like to postpone adding such clocks until we are finished with the rotational clock merge.

[christoff-buerger]

@bernhard-thiele I fixed point 2 (commit 73934511684d82d9de00c672d3c1c2a7f4fae717): The y-output of Modelica_Synchronous.IntegerSignals.NonPeriodic.IntegerChange now always is false for the first tick of the clocked-partition IntegerChange-instances are part of. Thus the implementation is:

if firstTick() then
  y = false;
else
  y = not(u == previous(u));
end if;

This change fixes the problem that direction_changed of rotational clocks is true for their first tick.

[christoff-buerger]

@bernhard-thiele Could you please review again and check if your concerns are now handled?

[bernhard-thiele]

Looks good :+1: The commit history has become fairly long, but due to the references to various issues it is probably not so easy to squash it in a good way, so let's just keept it as it is and merge it...

[christoff-buerger]

@bernhard-thiele Could you also close the respective issues please (issues #30, #32, #33, #34 and #36)? I do not have the permissions to do that. Thanks!