UltimateHackingKeyboard / firmware

Ultimate Hacking Keyboard firmware
Other
420 stars 66 forks source link

[Feature proposal] Macro validation #668

Closed kareltucek closed 1 year ago

kareltucek commented 1 year ago

Assume I write a solution that can validate smart macros, same as the config is validated via a dry run.

It would mean some nonnegligible amount of code clutter.

Do you find it worth the effort and the added maintenance burden (due to the code clutter)?

kareltucek commented 1 year ago

Pros:

Cons:

mondalaci commented 1 year ago

Depends on the amount of code clutter. In my view, about a hundred extra lines may be worth it, but not a thousand lines.

Increased power consumption is also a consideration. Would you run this validation on firmware bootup and configuration saves?

kareltucek commented 1 year ago

Like a one additional return somewhere in implementation of every command (~100 lines in total?), plus a ~50 line function to take care of traversing all the commands and dry-run them.

I assume I would run this validation on configuration saves, and maybe on bootup. It seems to me that config validation does not bring much added value on bootup and so if power consumption is a concern, I would leave that out.

mondalaci commented 1 year ago

Can you prefix the warnings/errors with the related macro names? If so, how much would prefixing increase clutter, assuming it'd increase it at all?

kareltucek commented 1 year ago

Current errors do contain macro name whenever issued while some macro is evaluated, so that would be the case during validation with no additional work/clutter.

Current error format is:

Error at {macroName} {actionIndex}/{commandAddress}: {errorMessage}: {command}

E.g.:

Error at tstErr 0/0: unrecognized command: asdfadsfasdf

(If you wish me to change it, let me know. I can imagine that for instance outputting line numbers instead of command addresses would be more user friendly, at the expense of some coding and clutter.)

mondalaci commented 1 year ago

How actionIndex and commandAddress differ from line numbers?

kareltucek commented 1 year ago

2023-08-17-152659_1501x465_scrot

mondalaci commented 1 year ago

Gotcha, thanks. Line numbers would be much more useful than command addresses. How much extra clutter to expose them?

kareltucek commented 1 year ago

Either

mondalaci commented 1 year ago

Picking the second option is a no-brainer here. Please make it happen.

kareltucek commented 1 year ago

Allright!

(I will simply assume it is a yes also to the original proposal of this ticket 😃.)

mondalaci commented 1 year ago

It is. :)

When there are smart macro related errors/warnings, configuration saves will always succeed, and then the errors and warnings will be put into the status buffer, right?

kareltucek commented 1 year ago

Yes!

mhantsch commented 1 year ago

Would you make Agent automatically retrieve the status after config save, so user gets an immediate response within Agent whether their macros are good? (Instead of having to look at the UHK display, then do some printStatus magic...)

kareltucek commented 1 year ago

Yes!

mondalaci commented 1 year ago

@mhantsch This will naturally happen as part of the planned Agent feature that exposes errors and warnings to the user.