Period option + control 'reset' behaviour

[bartbutenaers]

Hello @eisbehr-,

It is me again ... I have added two functionalities to your node. It would be great if you could review it, and give me some feedback. Thanks in advance !

Kind regards,
Bart

Change 1 - added a 'period' option

I'm using my NR flow for live streaming images from my IP camera. As soon as a pir sensor detects motion I want to create a movie of 10 seconds of the same room. To accomplish this, I have added a 'period' option to your node: it passes all messages in the first 10 seconds (after a reset message).

When a reset message arrives, it will again start counting from 0 to N seconds ...

Change 2 - control 'reset' message behaviour

When a 'reset' field is added, your node can be controlled smoothly. However, I needed to be able to specify whether this message should be skipped (i.e. it will never appear on the output) or not (i.e. it will appear on the output like all other non-reset messages). This way I can have pure reset messages (without a real payload), that are only relevant for resetting the node.

Remark: In your latest version, you processed the reset after you processed the message itself. E.g. following order:

1. Message with reset field arrives
2. A counter is incremented with 1
3. If counter equals a specified number, the message is being send
4. The counter is resetted (due to the reset field)

In my setup I did all reset handling at the start, to put all reset handling code at a single location (line 48):

           if( msg.reset) {
                initialize(false, node);
                
                if( node.resend !== true) {
                    // When a 'reset' message shouldn't be resended (on the output port), just skip it ...
                    return;
                }
            }

This means the reset is executed before the message processing. You can move the initialize call to the end of the function, if you want the same behaviour as in your latest version ...

[dkern]

Hello @bartbutenaers,

thank you for your PR and improvements! In general I like your ideas. But reviewing the code changes will take some time, and I'm not sure I will merge your commits. The reason is, that the commits are not really usefully documented and you changes a bit to much code your your changes as well. An you changed the behavior of the reset message order. That might be working for you, but changing this in an existing project should be done well. There could be users relay on this behavior. So I will review them and decide what to merge or not. Otherwise I would reimplement the features by myself ...

[bartbutenaers]

Hello @eisbehr-,

Ok, no problem. Next time I will create an (enhancement) issue first (to discuss the new required functionality), before I start coding.

Could you please explain "the commits are not really usefully documented", so I can take that into account in a future pull request?

Looking forward to your implementation of the new feature(s)!

Greetings,
Bart

[bartbutenaers]

Hello @eisbehr-,

Would you like me to create a new pull request? If you want I create a small pull request, that only adds the new period option. So there would be no rewrite of existing functionality! The reset functionality 'could' be added later...

Kind regards,
Bart