sml icon indicating copy to clipboard operation
sml copied to clipboard

unexpected_event seems to be affected by other event<> that have different from state

Open redboltz opened this issue 8 years ago • 7 comments

I think that the following code should print "unexpected". Because e1 is unexpected event for "state1"_s. But process_event(e1{}) doesn't seems to be matched to unexpected_event<_>.

http://melpon.org/wandbox/permlink/49iofZ2OJ5x9dP1v

#include <iostream>
#include <boost/sml.hpp>

namespace sml = boost::sml;                                                     

struct e1 {};
struct e2 {};

struct table {
    auto operator()() noexcept {                                               
        using namespace sml;
        // clang-format off
        return make_transition_table(
            *"state1"_s + unexpected_event<_> / [](auto const&) { std::cout << "unexpected" << std::endl; }
            ,"state2"_s + event<e1> / [](auto const&) { std::cout << "e1" << std::endl; }
        );
        // clang-format on
    }
};

int main() {
    sml::sm<table> sm;
    sm.process_event(e1{});
}

If I replaced "state2"_s + event<e1> with "state2"_s + event<e2>, the code works as I expected. http://melpon.org/wandbox/permlink/DvGbvDqU6pkiY1yc

I think that unexpected_event doesn't seems to care the from state in current implementation.

redboltz avatar Feb 15 '17 07:02 redboltz

You are probably right.

As the template process_event(...) is currently implemented it has two distinct specializations:

  • One is called with an event which is used in the transition-table (in any transition), and
  • the other is called with an event that is not used in the transition-table (at all).

The latter results in further processing the event with an unexpected_event wrapper around it, which might trigger one of the transitions with an unexpected_event<_>, as long as the source-state matches.

However, in your example above the event e1 is used in the transition-table, so the second specialization of process_event (which seems to be the only one to trigger unexpected_event<_> transitions) is never used.

This behavior (I would call it a bug) renders unexpected_event<_> pretty useless in transition-tables in which all allowed events are used in some transitions. It only prevents using events which should not be used at all in the transition-tables.

I assume, the first specialization of process_event needs an additional check whether a transition with the current event is defined for the current source-state or not. If yes, it could go on as before. If not, it should do the same as the second specialization.

@krzysztof-jusiak Am I right here? Do you know how such a check could be implemented?

DenizThatMenace avatar Jun 09 '17 09:06 DenizThatMenace

This still doesn't seem to work. Is there any workaround for this? Or is it intended behavior?

paulgessinger avatar Mar 21 '19 17:03 paulgessinger

This still doesn't seem to work. Is there any workaround for this? Or is it intended behavior?

You can always make the 'process_event' function return bool again and check the result after each call.

feelinfine avatar Oct 17 '19 07:10 feelinfine

You can always make the 'process_event' function return bool again and check the result after each call.

I don't think the return value dosen't solve the issue.

The issue is http://melpon.org/wandbox/permlink/49iofZ2OJ5x9dP1v should match

            ,"state2"_s + event<e1> / [](auto const&) { std::cout << "e1" << std::endl; }

In addition, the return type is void not bool. See https://wandbox.org/permlink/71AeD9vPo3Sd34eF

redboltz avatar Oct 17 '19 09:10 redboltz

In addition, the return type is void not bool.

That's right. The 'process_event' function looks like this

template <class TEvent, __BOOST_SML_REQUIRES(aux::is_base_of<TEvent, events_ids>::value)>
void process_event(const TEvent &event) {
    aux::get<sm_impl<TSM>>(sub_sms_).process_event(event, deps_, sub_sms_);
}

template <class TEvent, __BOOST_SML_REQUIRES(!aux::is_base_of<TEvent, events_ids>::value)>
void process_event(const TEvent &event) {
    aux::get<sm_impl<TSM>>(sub_sms_).process_event(unexpected_event<_, TEvent>{event}, deps_, sub_sms_);
}

and the inner aux::get<sm_impl<TSM>>(sub_sms_).process_event(event, deps_, sub_sms_) returns bool. So we can rewrite this function like this:

template <class TEvent, __BOOST_SML_REQUIRES(aux::is_base_of<TEvent, events_ids>::value)>
bool process_event(const TEvent &event) {
    return aux::get<sm_impl<TSM>>(sub_sms_).process_event(event, deps_, sub_sms_);
}
template <class TEvent, __BOOST_SML_REQUIRES(!aux::is_base_of<TEvent, events_ids>::value)>
bool process_event(const TEvent &event) {
    return aux::get<sm_impl<TSM>>(sub_sms_).process_event(unexpected_event<_, TEvent>{event}, deps_, sub_sms_);
}

and check return type after process_event call:


#include <stdexcept>
#include <memory>
#include <iostream>

#include "sml.hpp"

namespace sml = boost::sml;

struct ev1 {};

auto state1 = sml::state<class state1>;
auto state2 = sml::state<class state2>;
auto err_state = sml::state<class err_state>;

struct transitions
{
   auto operator()() const noexcept
   {
      using namespace sml;

      auto event1_act = [] { std::cout << "event1"; };
      auto err_act = [] { std::cout << "error"; };   //never called

      return make_transition_table
      (
         *state1  + unexpected_event<_>   / err_act      = state2,
         state2   + event<ev1>            / event1_act   = err_state
      );
   }
};

int main()
{
   sml::sm<transitions> sm;

   auto rv = sm.process_event(ev1{});   //no such transition

   if (!rv)
      std::cout << "no transition";

   return 0;
}

I know it's not a solution, but maybe a workaround for someone. Actually we need something like a 'no_transition' callback

feelinfine avatar Oct 18 '19 07:10 feelinfine

@feelinfine , thank you for clarifying. Now I understand what you mean :)

redboltz avatar Oct 18 '19 08:10 redboltz

@redboltz Sorry for my bad english knowledge)

For me the main problem was to handle invalid transitions from states, which are present in transition table. But now I see that simply using event<_> instead of unexpected_event<_> works fine

#include <iostream>
#include "sml.hpp"

namespace sml = boost::sml;

struct ev1 {};
struct ev2 {};

auto state1 = sml::state<class state1>;
auto state2 = sml::state<class state2>;

struct transitions
{
   auto operator()() const noexcept
   {
      using namespace sml;

      return make_transition_table
      (
         *state1 + event<_>   / [] { std::cout << "no_transition "; }  = state1,
         state1 + event<ev1>  / [] { std::cout << "event1 "; }         = state2,
         state2 + event<ev2>  / [] { std::cout << "event2 "; }         = state1
      );
   }
};

int main()
{
   sml::sm<transitions> sm;

   sm.process_event(ev2{});   //no transition
   sm.process_event(ev1{});   //ok
   sm.process_event(ev2{});   //ok

   return 0;
}

feelinfine avatar Oct 18 '19 21:10 feelinfine