- From: Ate Douma <ate@douma.nu>
- Date: Wed, 02 Apr 2014 16:39:02 +0200
- To: www-voice@w3.org
On 02-04-14 16:10, Jim Barnett wrote:
> If getTargetStates isn't needed to dereference history states, then it can be
> dropped altogether, because it is returning the same thing as transition.target.
Indeed. Which is how I'm using it.
>
> - Jim
> On 4/2/2014 5:27 AM, Ate Douma wrote:
>> On 02-04-14 00:10, Jim Barnett wrote:
>>> Ate,
>>> There is a problem if getTargetStates does not dereference history states.
>>> computeEntrySet calls it to produce the initial list of states for
>>> addDescendentStatesToEnter to expand. If getTargetStates doesn't dereference
>>> history states, they will get added to the statesToEnter list before
>>> addDescendentStatesToEnter gets called - and history states should not be on
>>> statesToEnter. Furthermore, computeExitSet calls getTargetStates, and it will
>>> also need to know what the actual dereferenced target state set is.
>>
>> Right. And I realize I already solved this differently, and IMO elegantly, in
>> my implementation :)
>>
>> The 'hint' leading to my implementation was the fact that
>> addDescendantStatesToEnter (also) stores the state parameter in statesToEnter.
>> And is expected to do so as it recursively is invoked for derived states which
>> were not yet initially added to the statesToEnter.
>>
>> What I thus did instead is the following (in pseudo language):
>>
>> procedure computeEntrySet(transitions, statesToEnter, statesForDefaultEntry)
>> enterableTargets = new OrderedSet()
>> historyTargets = new OrderedSet()
>> for t in transitions:
>> for s in getTargetState(t.target)):
>> if isHistoryState(s):
>> historyTargets.add(s)
>> else
>> enterableTargets.add(s)
>> for s in enterableTargets:
>> addDescendantStatesToEnter(s,statesToEnter,statesForDefaultEntry)
>> for h in historyTargets:
>> addDescendantStatesToEnter(h,statesToEnter,statesForDefaultEntry)
>> // unmodified below
>> for t in transitions:
>> ancestor = getTransitionDomain(t)
>> for s in getTargetStates(t.target)):
>> addAncestorStatesToEnter(s, ancestor, statesToEnter,
>> statesForDefaultEntry)
>>
>> This way, no history state is added to enterableStates because
>> addDescendantStatesToEnter only does this for non-history states.
>>
>> Note that addAncestorStatesToEnter will work just fine with a history state as
>> parameter.
>>
>> Finally, concerning computeExitSet: this also isn't a problem in my
>> implementation because I already 'compile' the source of a history transition
>> to that of its parent state, and actually (thus) can 'compile' the
>> transitionDomain and the targets of a transition at document load time.
>>
>> In pseudo-language in the algorithm, this also can easily be catered for if
>> you update getTransitionDomain as follows:
>>
>> function getTransitionDomain(t)
>> tstates = getTargetStates(t.target)
>> if not tstates
>> return t.source
>> state = t.source
>> if isHistoryState(t.source):
>> state = t.source.parent
>> if t.type == "internal" and isCompoundState(state) and
>> tstates.every(lambda s: isDescendant(s,state)):
>> return state
>> else:
>> return findLCCA([state].append(tstates))
>>
>> The above can further be optimized by filtering out the history targets from
>> being passed to the lambda expression or from the findLCCA(stateList), but it
>> isn't necessary either.
>>
>> With the above minor improvements, the getTargets(transition) as well as the
>> getTransitionDomain(transition) procedures can remain 'stateless' (not history
>> state aware) and can be compiled at load time.
>> If you add history dereferencing to getTargets(transition) this no longer will
>> be possible, so IMO is something we should try not to do.
>>
>>>
>>> It's a big gap in the algorithm that getTargetStates isn't defined - I'm
>>> assuming that it was in an earlier version, and got lost somewhere.
>>>
>>> In any case we need multiple fixes so that: 1) history states don't end up on
>>> the statesToEnter list and 2) default history executable content gets recorded.
>>
>> See my above proposed changes, AFAICT it solves 1) elegantly and then 2) no
>> longer is an issue (already solved by the earlier update to the algorithm).
>>
>> Regards,
>> Ate
>>
>>>
>>> - Jim
>>> On 4/1/2014 2:45 PM, Ate Douma wrote:
>>>> On 29-03-14 20:26, Zjnue Brzavi wrote:
>>>>> On Sat, Mar 29, 2014 at 1:24 PM, Jim Barnett <1jhbarnett@gmail.com
>>>>> <mailto:1jhbarnett@gmail.com>> wrote:
>>>>>
>>>>> Zjnue,
>>>>> I agree that we need more tests, but why would defaultHistoryContent
>>>>> need
>>>>> to get passed to getTargetStates? It's just an accessor function.
>>>>> Though I
>>>>> do think we have to consider how getTransitionDomain and findLCCA should
>>>>> handle history states. They ignore them at the moment, and _maybe_ that's
>>>>> ok...
>>>>>
>>>>> - Jim
>>>>>
>>>>>
>>>>> Hi Jim,
>>>>>
>>>>> Perhaps as quick background, my implementation follows (atm) the proposed
>>>>> algorithm pretty much verbatim, in order to aid debugging for cases like this
>>>>> and to make getting started with SCXML a little easier for myself and others.
>>>>>
>>>>> Simply put, the recently published changes to the algorithm for
>>>>> defaultHistoryContent support are not satisfying test579.
>>>>>
>>>>> While the getTargetStates function seems pretty harmless, it is also
>>>>> hisory-aware and can redirect a transition target.
>>>>
>>>> Hi Zjnue,
>>>>
>>>> Sorry for chiming in this late, and going back to this earlier message, but
>>>> I'm puzzled by the above statement.
>>>>
>>>> I can find no indication in the algorithm nor in the specification text that
>>>> the getTargetStates function is supposed to do anything more than just
>>>> returning the direct target(s) of a transition.
>>>>
>>>> It seems to me that the problems you encountered are actually *caused* by the
>>>> the history-awareness and dereferencing in your getTargetStates implementation.
>>>>
>>>> AFAICT the pseudo algorithm expects it will take care of the history
>>>> dereferencing itself in the addDescendantStatesToEnter procedure.
>>>> If you do this upfront in you getTargetStates implementation, the
>>>> addDescendantStatesToEnter never will get to 'see' a history state as it is
>>>> supposed to, and neither can it then handle the default history content as
>>>> just was added.
>>>>
>>>> I just completed the implementation of the current algorithm for Apache
>>>> Commons SCXML (although not yet committed), and I now can successfully run
>>>> test579, so it seems to me the proposed change simply works as expected.
>>>>
>>>> But maybe I'm overlooking some other specification requirements for
>>>> getTargetStates?
>>>> I'd appreciate some pointers in that case :)
>>>>
>>>> Regards, Ate
>>>>
>>>>
>>>>> In the case of test579, the <initial> transition, targets <history id="sh1">,
>>>>> but is effectively redirected to <state id="s01" >, skipping any executable
>>>>> content of <history> along the way.
>>>>>
>>>>> Please take a look at my code for getTargetStates and getTargetState below
>>>>> [1].
>>>>>
>>>>> It is only with the following two lines present in getTargetState:
>>>>> if( defaultHistoryContent != null &&
>>>>> h.parent.exists("id") )
>>>>> defaultHistoryContent.set(h.parent.get("id"),
>>>>> h.transition().next());
>>>>> ..that 579 passes.
>>>>>
>>>>> As mentioned, I did not look at it in depth and will not be able to spend more
>>>>> time on it over the next few days unfortunately.
>>>>>
>>>>> So in summary, after adding the published additions to the algorithm, test579
>>>>> did not pass.
>>>>> It was only after the changes to getTargetStates that it did.
>>>>> Hope this helps for now.
>>>>>
>>>>> Thanks,
>>>>> Zjnue
>>>>>
>>>>> [1]
>>>>>
>>>>> function getTargetStates( node : Node, ?defaultHistoryContent :
>>>>> Hash<Iterable<Node>> ) : List<Node> {
>>>>> var l = new List<Node>();
>>>>> if( !node.exists("target") )
>>>>> return l;
>>>>> var ids = node.get("target").split(" ");
>>>>> var top = node;
>>>>> while( top.parent != null && !top.isTScxml() )
>>>>> top = top.parent;
>>>>> for( id in ids ) {
>>>>> var ts = getTargetState(top, id, defaultHistoryContent);
>>>>> for( tss in ts )
>>>>> l.add( tss );
>>>>> }
>>>>> return l;
>>>>> }
>>>>>
>>>>> function getTargetState( s : Node, id : String, ?defaultHistoryContent :
>>>>> Hash<Iterable<Node>> ) : List<Node> {
>>>>> if( s.get("id") == id )
>>>>> return [s].toList();
>>>>> else {
>>>>> for( child in s.getChildStates() ) {
>>>>> var ss = getTargetState(child, id, defaultHistoryContent);
>>>>> if( ss != null )
>>>>> return ss;
>>>>> }
>>>>> for( h in s.history() ) {
>>>>> var hh = getTargetState(h, id);
>>>>> if( hh != null ) {
>>>>> if( defaultHistoryContent != null &&
>>>>> h.parent.exists("id") )
>>>>> defaultHistoryContent.set(h.parent.get("id"),
>>>>> h.transition().next());
>>>>> return historyValue.exists( h.get("id") ) ?
>>>>> historyValue.get( h.get("id") ) :
>>>>> getTargetStates( h.transition().next(),
>>>>> defaultHistoryContent );
>>>>> }
>>>>> }
>>>>> }
>>>>> return null;
>>>>> }
>>>>>
>>>>>
>>>>>
>>>>> On 3/28/2014 8:51 PM, Zjnue Brzavi wrote:
>>>>>>
>>>>>> I've added test579 to the suite. Let me know if there are problems
>>>>>> with it or if you think there need to be more/different tests.--
>>>>>>
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Not had a very good look yet, but it appears we'll need to pass
>>>>>> defaultHistoryContent to getTargetStates as well, as it is another area
>>>>>> where the algorithm is skipping past some history elements without
>>>>>> considering their executable content. This diff passes test579 :
>>>>>> https://github.com/zjnue/hscxml/commit/4b9091470d3c7c1995e6b41900535e0437ec3eb4
>>>>>>
>>>>>> Another test or 2 on the subject is probably a good idea.
>>>>>>
>>>>>> Best regards,
>>>>>> Zjnue
>>>>>
>>>>> --
>>>>> Jim Barnett
>>>>> Genesys
>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>>
>
Received on Wednesday, 2 April 2014 14:39:33 UTC