Please suppress redundant "self->state = NULL" assignments.

[David-Gould]

I'd like to use this for very small 8 bit MCUs like STM8 and Pauduk with the SDCC compiler. Unfortunately SDCC does not optimize away the dead assignments, which bloats the generated code considerably.

If I were greedy, I'd also ask for an option to declare the type of the state variable so I could use uint8_t instead of int.

BTW, I really like the description language and the generated code, so clear and clean.

[clnhlzmn]

Removing the redundant assignments isn't a problem.

Currently the state variable is a function pointer. It would take more significant refactoring to change that to an int of minimum size, but it can be done.

I will look into those things.

You can already change the type of the ctx member of the state and event types using the -m and -e command line options, respectively. Maybe it would be nice, for your use case, to specify that those members be omitted all together.

[David-Gould]

Ah, I skimmed the definition too quickly and missed that state is a function pointer and thought it was an int. It's fine as is. Sorry for the confusion.

It might be useful to optionally be able to omit the ctx members from states or events.

[clnhlzmn]

I don't think you're wrong to say the state variable could be optimized. I'm not sure how function pointers are implemented on STM8 and Pauduk, but I'm guessing they're at least 16 bits where in most cases the state variable could easily fit in 8 bits. Then the dispatch_event function would use a switch statement on the state variable, rather than calling the state function. I'm not sure what effect that would have on code size, perhaps it would increase. If it did increase code size perhaps it wouldn't be worth saving one byte of RAM. If it caused a decrease in code size I think it would be a good change.

[David-Gould]

I'm not sure which would be better, these size MCUs tend to have more code space than data space, for example 1K ram, 8K flash or 256b ram, 2K flash. But, I suspect there will not be so many states that the extra byte matters and the case statement might be larger than the call via pointer anyway. Without some real application code to look at it's hard to justify a change.

It would be nice to optionally eliminate the tests at function entry:

if (!self || !event) return -1;

Maybe put that under the same option as suppressing the redundant state assignment. It's nice to have safety checks, but at least the static functions are only likely to be called from generated code I think it's ok to leave them out except as a check on the code generator. Maybe generate them as ASSERTs?

[clnhlzmn]

Without some real application code to look at it's hard to justify a change.

I was thinking I would make the change and compile some examples to try to measure the difference. See if it's worth it.

It would be nice to optionally eliminate the tests at function entry

Yes those can be eliminated in the static state functions because as you said they're only called from process_event which already does the same check.

[David-Gould]

I don't know, I'm looking at stm8 with the oven example and the calling is pretty cheap, the case statements are just linear chains of compare and branch and will be slower for sure once there are more than a few cases.

I took the liberty of hand editing the generated oven.c to remove the extra assignments and tests and to rearrange the test in the dispatch into one expression. I think I should update to latest sdcc, the one I have is pretty stupid about redundant loads and I would not want you to contort your code if they are going to fix the issue anyway.

I'll play with it some more and share what I find out.

[clnhlzmn]

Ok thanks.

I think changes to the state variable like this:

self->state = NULL;
self->state = oven_closed_cooking;

where nothing happens between should definitely be eliminated.

The reason they are like that is in the case the transition has an action you get something like this:

self->state = NULL;
some_action(self, event);
self->state = oven_closed_cooking;

This is based on the UML state machine specification which says actions happen after all states have exited and before all states have been entered. In the future I might add an api for the user to query the machine's current state and to match the UML state machine specification if the machine state is queried from within a transition action function it should show that it is not currently in any state.

I hope that make sense. It doesn't mean that the redundant double assignment shouldn't be eliminated I'm just trying to explain why it's like that (because I didn't optimize that distinction in the code generator).

[clnhlzmn]

It would be nice to optionally eliminate the tests at function entry:

if (!self || !event) return -1;

Fixed in 2543b5ea1a4ca7292fbf921e12484ae0d689b9d9.