-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Plugin P167 IKEA Vindstyrka #4991
Conversation
@andibaciu If you add the text |
In the start post of the pull request :) |
for standalone sensirion sen54 and sen55
const uint8_t i2cAddressValues[] = { P167_I2C_ADDRESS_DFLT }; | ||
if (function == PLUGIN_WEBFORM_SHOW_I2C_PARAMS) | ||
{ | ||
if (P167_SEN_FIRST == event->TaskIndex) // If first SEN, serial config available |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why so complex with checking the first taskindex?
This is prone to cause a lot of issues and should only be done with a very specific and very good reason.
Also below with PLUGIN_INIT
you're doing stuff different if it is not the first task.
I will explain in a separate post why this is a bad idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are so many reason for this type of implementation:
- All SEN5x devices are the same i2c address 0x69 .... so this limitation permit ONLY one device on the bus
- for IKEA Vindstyrka it's necessary to minimize reading pull i2c communication (because multimaster problems) and in this case ONLY for first task i activate pin interruption monitoring SCL i2c communication and read ALL parameters from SEN5x. If you want to read more then 4 parameters from the same device you need to ad another one with different name and get from first task requested parameters (without activate another interrupt and make another pull i2c request on the bus)
- for what reason somebody will add more then 2 task plugin and after that will disable the first one ???
- i see this kind of implementation on other plugins (it's not my creation)
Thank You!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2. If you want to read more then 4 parameters from the same device you need to ad another one
That is not necessary, you can implement the PLUGIN_GET_CONFIG_VALUE
function, that allows to fetch as many values from a plugin as needed, as demonstrated in many plugins (search the source code via VSCode to find examples), from very simple implementations like P021 (Level Control), to 'tricky' ones like P137 (AXP192) and somewhat complicated implementations like P095/P116/P131/P141 (Displays using AdafruitGFX_helper).
This PLUGIN_GET_CONFIG_VALUE
functions is used exactly like the regular [<taskname>#<valuename>]
variables in rules.
For sending data via a Controller you can use as many Dummy Device plugins as desired, that receive their data via rules (TaskValueSet
/TaskValueSetAndRun
) from that 1 P167 instance, or use the Publish
and SendToHTTP
families of commands.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @tonhuisman for this short description of PLUGIN_GET_CONFIG_VALUE.
For further plugin i'll keep in mind to use this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can technically use multiple I2C devices on the same I2C address, by using I2C multiplexers.
There is a number of devices where you may get multiple values at once, without asking for them, like GPS.
For those you may need a single GPS string parser instance which can be used by multiple tasks to get over 4 values.
If you need to specifically trigger a measurement and later collect its reading, then it also makes sense to have some instance running which can handle the interactions with the sensor.
But this should not be done in the ino file.
It is best to have a shared pointer which is a member of the PluginStructs
.
If you try to do this manually in the plugin's .ino file, you will override a lot of ESPEasy-core code and -checks which will make this really hard to debug and to maintain as it is working completely different from all other plugins.
Maybe you can take a look at the Eastron plugin as this one is using a separate class to take in requests for registers to read from various tasks and returns the results to the appropriate task values.
The 1-wire plugin (P004) is also doing something special as it needs to combine several concurrent requests while using the same GPIO pin for all connected 1-wire sensors.
The GPS plugin deals with those "free" extra values by adding some "virtual" taskvalues (PMSx003 does the same) as all other values which are received anyway are made available via standard taskvalues even though those were never defined as task value output.
What is set as task value will be sent to any connected controller.
All others may be accessible via rules so you can copy those to a dummy task which can then send it to a controller if needed.
About the pin description....
Every I2C plugin/task is using the same GPIO pins for I2C.
If that needs to be dealt with, then this should be done in ESPEasy core code, not in the plugin code.
But for now, do not try to work around checks etc. which makes the plugin "special" as it is an absolute nightmare to maintain it.
There are enough examples of plugins where you need to read way more than 4 values from the same sensor, so please follow those instead of re-inventing the wheel.
I have spent a lot of time in making the plugins act more uniform, so please help keep the number of variants to a minimum.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK @TD-er , I understand and I try to remake plugin construction to meet standard plugin requirements.
You're making it really complex by adding checks for being the first task of this type. So why do you need this? One example where this will fail:
This will not call PLUGIN_INIT for any of the remaining tasks thus as a result it will no longer work until you either reboot or call And there are probably other issues too. |
I have 2 SEN55 here, ready to be connected to 2 WT32-SC01, currently running on ESP_Easy_mega_20240203_max_ESP32_16M1M Feb 3 2024. |
In the file attached you have all description you need.
ESPeasy plugin for IKEA VINDSTYRKA.pdf
Resolves #4794