LennartHennigs / SimpleFSM

Arduino/ESP library to simplify setting up and running a state machine.
MIT License
66 stars 15 forks source link

Improve GraphViz output by using named events #18

Open marcelstoer opened 7 months ago

marcelstoer commented 7 months ago

Including GraphViz/dot support is a very nifty idea, thanks!

The only issue I see is the lack of named events. The example at https://github.com/LennartHennigs/SimpleFSM/blob/master/README.md#graphviz-generation would be a lot more legible IMO if instead of "(ID = 1)" it said "press button". I understand this won't be possible out-of-the-box as events are defined by number rather than by name (usually through an enum).

However, what if we could pass something like an "event name array" to the SimpleFSM constructor (or through a setter)? Use a preprocessor macro to generate said array or hand-craft it.

enum events {
  button_clicked,
  button_double_clicked,
};

#define IDNAME(name) #name
const char* eventNames[] = {IDNAME(button_clicked), IDNAME(button_double_clicked)};

void setup() {
  ...
  fsm.setEventNames(eventNames);
  ...
}

(borrowed from here)

LennartHennigs commented 7 months ago

Hey Marcel, danke für die Blumen.

To your question – as you'd only need the names in the output, I was thinking if it'd make sense to pass them as a parameter to the getDotDefinition(). What do you think?

marcelstoer commented 7 months ago

Sure, why not. It would be a little less invasive.

LennartHennigs commented 7 months ago

Ok, thx.

A while ago I also considered to name them in order to be able to do something like this:

Transition transitions[] = {
  Transition('from', 'to, light_switch_flipped),
  Transition('to', 'from' light_switch_flipped)
};

Which would make defining transitions more readable and easier to understand. But that would require some larger changes to the code. I'll think about it a little more. And add it to my todo list.

marcelstoer commented 7 months ago

pass them as a parameter to the getDotDefinition().

The obvious downside of this approach is that you would need to redo the whole dot_definition that you are currently aggregating behind the scenes on add(). If you allow the transition names to be passed (optionally) to the constructor you'd only need to change the _addDotTransition methods.

I also considered to name them in order to be able to do something like this:

That would also be an option. However, I am not so sure it would really make the code more legible. If I name my enum values (i.e. transition ids) expressively enough, I get a very well legible transition definition.

marcelstoer commented 5 months ago

Sigh, I feel like such a fool 🙈 I only just realized that I can produce more legible diagrams using the existing code. I just have to name my transitions. Feel free to close this, if you don't plan to pursue the idea of "named events".

LennartHennigs commented 5 months ago

😀 no problem. Still have it on my list. Did not find the time yet. Will leave it open for now.