[P076] Added the ability to read reactive power, apparent power and energy

[dsiggi]

Hi,
I added some new functions to the plugin:

• Added the ability to read reactive power
• Added the ability to read apparent power
• Added the ability to read energy
• Energy counter rest with command "hlwresetenergy"

I also fixed a bug that prevented the plugin from being built with PLUGIN_076_DEBUG enabled.

[tonhuisman]

It seems you haven't formatted the source using Uncrustify. That's highly recommended, as it improves readability and alignment, and it also adds round braces where advisable.

[tonhuisman]

These strings are used as value names, but the rules parser doesn't handle/accept spaces in value names, so the remaining spaces should probably also be replaced by underscores.
And I'd expect the previous value names Voltage, Current, Power and PowerFactor, to be available, and preferably as the defaults (would require swapping case 3 and case 5, and also some other code adjustments), to not break existing installs and rules based on that.

[tonhuisman]

Can you also add a short description of your changes to the Changelog, at the top of the file (newer entries on top of the list)?

[dsiggi]

Hi,
I did all changes but I have one problem.

Okay forgot everything under this part. my current calibration value was wrong.

The measurement of the current is not working. The current is always 0. So I flashed back an build with the original plugin but there the current is also every time 0.

So I added a the logAddMove command at after line 389 to see if the measurement is running. But the log is never shown in the log.

I think the problem is that the variable p076_read_stage is every time 0. The online line where it is written is in 478, but to 0.

There is also a commented out block (lines 422 - 424) where it will be set to 1.

[TD-er]

See what I did for the P082_GPS in Plugin_082_valuename
There I have the display string and a task value string.
The display string is shown in the combobox when selecting the output value.
The non-display string is the shorter taskvalue name.
So maybe that's also more useful here as you would otherwise get really long strings for [taskname#taskvaluename] which may be an issue when showing on some display as most line fields for display plugins have a limited string length.
Also it will probably mess up the layout on the "Devices" tab.

[dsiggi]

You're right, that wouldn't look good on the overview. I have adapted the strings based on the P082 plugin.

[dsiggi]

I have found a problem with the device I am using for testing (Shelly Plus Plug S) . The BL0937 is located behind the relay. When I switch off the relay, the chip does not measure voltage, current or power. This is a problem because when PLUGIN_READ is executed all functions return a valid=false. This means that incorrect values are displayed.

I would remove all the "if (valid)"-blocks and set success=true directly. This way a 0 is displayed for the values.

What do you think?

[tonhuisman]

I have found a problem with the device I am using for testing (Shelly Plus Plug S) . The BL0937 is located behind the relay. When I switch off the relay, the chip does not measure voltage, current or power. This is a problem because when PLUGIN_READ is executed all functions return a valid=false. This means that incorrect values are displayed.

I would remove all the "if (valid)"-blocks and set success=true directly. This way a 0 is displayed for the values.

What do you think?

I think that returning 0 for these values when the relay is off is a bit silly, but probably a deliberate design choice to indicate you're measuring the output values, not the input values. And only Voltage would be valid anyway when the relay is off. (AFAIR, the Sonoff POWR3xxD/THR3xxD has a similar behavior)

You can add a setting, and also add a default for that setting when fetching the presets, so the checks can optionally be ignored to show 0 for the values. That would make it most flexible.

[dsiggi]

I have found a problem with the device I am using for testing (Shelly Plus Plug S) . The BL0937 is located behind the relay. When I switch off the relay, the chip does not measure voltage, current or power. This is a problem because when PLUGIN_READ is executed all functions return a valid=false. This means that incorrect values are displayed.
I would remove all the "if (valid)"-blocks and set success=true directly. This way a 0 is displayed for the values.
What do you think?

I think that returning 0 for these values when the relay is off is a bit silly, but probably a deliberate design choice to indicate you're measuring the output values, not the input values. And only Voltage would be valid anyway when the relay is off. (AFAIR, the Sonoff POWR3xxD/THR3xxD has a similar behavior)

You can add a setting, and also add a default for that setting when fetching the presets, so the checks can optionally be ignored to show 0 for the values. That would make it most flexible.

That's a good idea. I hope I can implement it next week.

[dsiggi]

I have found a problem with the device I am using for testing (Shelly Plus Plug S) . The BL0937 is located behind the relay. When I switch off the relay, the chip does not measure voltage, current or power. This is a problem because when PLUGIN_READ is executed all functions return a valid=false. This means that incorrect values are displayed.
I would remove all the "if (valid)"-blocks and set success=true directly. This way a 0 is displayed for the values.
What do you think?

I think that returning 0 for these values when the relay is off is a bit silly, but probably a deliberate design choice to indicate you're measuring the output values, not the input values. And only Voltage would be valid anyway when the relay is off. (AFAIR, the Sonoff POWR3xxD/THR3xxD has a similar behavior)
You can add a setting, and also add a default for that setting when fetching the presets, so the checks can optionally be ignored to show 0 for the values. That would make it most flexible.

That's a good idea. I hope I can implement it next week.

As I understand it, each option that is saved must be linked to a PCONFIG() number. There are 8 of these per plugin (0-7).

Unfortunately, all 8 are already used here.

0 = output value 1
1 = output value 2
2 = output value 3
3 = output value 4
4 = pin number SEL
5 = pin number CF
6 = pin number CF1
7 = device pin setting

How or where could I save the information whether 0 is displayed for a non-valid measurement?

[tonhuisman]

As I understand it, each option that is saved must be linked to a PCONFIG() number. There are 8 of these per plugin (0-7).

Well, yes and no 😃
There are 8 PCONFIG values (int16_t), 4 PCONFIG_LONG values (int32_t) (in a union with PCONFIG_ULONG (uint32_t), using the same storage space) and 4 PCONFIG_FLOAT values (float). And also 16 ExtraTaskSettings.TaskDevicePluginConfigLong values (long) and 16 ExtraTaskSettings.TaskDevicePluginConfig values (int16_t), but these aren't used often as they require an extra call to ensure they are loaded in memory.
I often use the PCONFIG_ULONG values to store 1, 2, 3, 4, 8 or 16 bit values. For 1 bit we use bitWrite()/bitRead(), and sometimes bitSet, functions, and for getting/setting the 2, 3, 4 and 8 bit values there are support functions get2BitFromUL/set2BitToUL etc. See f.e. P131_data_struct.h.

For this use-case I suggest to go the PCONFIG_ULONG route, as they have the same availability as the PCONFIG variables.