Dummy Data Injector#5566
Conversation
| ":ref:`P079_page`", "✓", "✓", "P079" | ||
| ":ref:`P146_page`", "✓", "✓", "P146" | ||
| ":ref:`P152_page`", "✓", "✓", "P152" | ||
| ":ref:`P187_page`", "✓", "", "P187" |
There was a problem hiding this comment.
This file is re-generated when the PR is merged, so no need to include it in your PR. Unfortunately, the .gitignore entry for this file is ignored 🤓
|
@SuksAE Thanks for your PR! It's a very good first PR 😃 I've added a few comments, all with the intent of making this even better 👍 we are a friendly bunch, so don't hesitate to ask for more info if something isn't clear, or you have a specific reason for doing things different from what's suggested. |
Indeed I have a question: I just found out that PLUGIN_SET_DEFAULTS is called before PLUGIN_INIT. Is this intentional, or is this an error? I mean I can't set default values before the plugin is initialised, or am I wrong? |
PLUGIN_SET_DEFAULTS is called only once, right after the new plugin instance is added to the Devices list. And never again after that. This is the place to set initial defaults for your plugin. PLUGIN_INIT is called every time the plugin is enabled, and should return |
|
One more questions I hope you can help me: As I am not a git expert - how can I remove the file docs/source/Plugin/_plugin_sets_overview.repl from the PR? |
|
except from removing _plugin_sets_overview.repl from the PR, I hope I have adressed all your findings... please re-check... |
By restoring it to its original contents (checkout |
|
done. please check... |
| break; | ||
| case P187_OUTPUT_TRAPEZ: | ||
|
|
||
| if ((P187_time[i] * P187_param[i].param2 / 360.0f) < P187_param[i].param4) // rising edge |
There was a problem hiding this comment.
Maybe also check to see if param4 is nearly 0.
See the functions in ESPEasy_math.h For example essentiallyZero.
| } | ||
| else if ((P187_time[i] * P187_param[i].param2 / 360.0f) < (P187_param[i].param4 + P187_param[i].param3 + P187_param[i].param5)) // falling edge | ||
| { | ||
| UserVar.setFloat(event->TaskIndex, i, |
There was a problem hiding this comment.
both are integers...
Well that's an even simpler check :)
|
Why not naming the plugin something with "Function Generator" in the name? |
Because then I would not be happy with the current implementation - I would want more functionality of a function generator (more different patterns, arbitrary output data read from a .wav file in LittleFS, controllable via network commands, ...) and a timing resolution of way less than a second. |
|
revised time handling... |
| int t_ms; // time within period in ms | ||
|
|
||
| if (now < periodStart[i]) { // check for overflow (happens each ~49 days) | ||
| t_ms = (now + (ULONG_MAX - periodStart[i])); |
There was a problem hiding this comment.
See the timeDiff function in src/src/Helpers/ESPEasy_time_calc.h
Or timePassedSince, but since you want to use the same now timestamp (which is perfectly fine) I guess using timeDiff is the better choice here.
| /* | ||
| Configuration and working variables are implemented as is because: | ||
|
|
||
| -- excerpt from https://github.com/letscontrolit/ESPEasy/pull/5566#issuecomment-4739927575: | ||
|
|
||
| PLUGIN_SET_DEFAULTS is called only once, right after the new plugin instance is added to the Devices list. | ||
| And never again after that. This is the place to set initial defaults for your plugin. | ||
|
|
||
| PLUGIN_INIT is called every time the plugin is enabled, and should return true if all is OK, | ||
| or false if there is an error causing the plugin to stay disabled. | ||
|
|
||
| -- excerpt end | ||
|
|
||
| So in PLUGIN_SET_DEFAULTS, the persistent configuration values in flash are initialized. | ||
| This flash data is used in PLUGIN_WEBFORM_LOAD to provide data for the UI generation. The | ||
| flash data is updated in PLUGIN_WEBFORM_SAVE when new configuration values are submitted by | ||
| the user. | ||
|
|
||
| Afterwards, when the plugin task instance is enabled via the UI (and output data starts | ||
| to be generated), a working copy of the configuration values is created by loading them | ||
| from flash to RAM. | ||
| Stored together with the volatile variables, this data is used for fast processing of | ||
| the signal generation code. | ||
| */ | ||
|
|
There was a problem hiding this comment.
Not sure why you have included this quit elaborate text? Smells like it's AI generated, as it's 'how things work' in ESPEasy, no need to 'document' that in the source code, AFAICS.
3 participants
Here is the pull request for the Dummy Data Injector Plugin.
I did not know what to use for the documentation status (|P187_status| in
file 'docs/source/Plugin/_plugin_substitutions_p18x.repl') - maybe you have
a suggestion...
Thanks