eworm-de / routeros-scripts

a collection of scripts for MikroTik RouterOS
GNU General Public License v3.0
1.27k stars 285 forks source link

Allow sms forward actions #30

Closed bubenkoff closed 1 year ago

eworm-de commented 1 year ago

Are you aware that the script sms-action exists? Why reinvent and duplicate code for sms-forward?

bubenkoff commented 1 year ago

@eworm-de thank you for mentioning it, yes I do, but it's not the same thing sms-actions relies on the built-in sms-in message format This approach allows parsing arbitrary SMS and acting based on those the use case I use it for currently is to order the traffic bundles automatically when the warning SMS about 80% usage is received. It then sends an SMS to the provider to order a new package.

eworm-de commented 1 year ago

Ah, got the idea. 😜 I will have another look with some spare time.

eworm-de commented 1 year ago

I am very unhappy with the naming. As mentioned we have sms-action. That does something different, in facts acting on the received sms. The primary use of sms-forward is to forward sms messages, with the new functionality hooking in. So this is my proposal: Let's put "Hook" in naming.

Also I do not like to recycle the allowed-number property here. Let's keep that for sms-action. I would prefer to have an allowed number for each hook, preferable as regular expression so matching several numbers is possible.

So I think something like this could serve the configuration:

:local SmsForwardHooks {
  { match="magic string";
    allowed-number="1234";
    command="/system/script/run ..." };
# add more here...
};

Do you think that's reasonable?

bubenkoff commented 1 year ago

good points, will update the PR

bubenkoff commented 1 year ago

@eworm-de your points are now implemented, please re-review BTW: can you please use the standard github code review tools to comment on the code?

eworm-de commented 1 year ago

Merged in ea09a18d3f05b20ff7f1ec25aea74f32d68ca5e0, with some modifications. Thanks!