Support Shelly 2.5 #1827

tonilopezmr · 2019-07-14T22:12:32Z

📌 References

Issue: fixes Support for Shelly 2.5 #1698

💣 Issue

Add new device Shelly 2.5 support following Shelly 2.5 tasmota support.

🎩 What is the goal?

Integrate Shelly 2.5 with espurna to control the relay, control built-in led, read wall switch changes, get voltage, current, and temperature.

📝 How is it being implemented?

Turn relay on/off, Relay 1 GPIO4, Relay 2 GPIO15
Read switch state, Relay 1GPIO13, Relay 2 GPIO5
Add new ADE9753 sensor reading by I2C at address 0x38, SDA GPIO12 and SCL GPIO14
- We get voltage, current relay 1 and current relay 2
- I didn't use i2c.ino because I see only support for reading and write up to uint16, Do I need to add uint32 functions version?.
- Add three different instances of this sensor with a _relay configuration which could be ALL, RELAY_1 or RELAY_2 (this was my first attempt) to show (RELAY 1 + RELAY 2) consumption, RELAY 1 and RELAY 2. (See on the picture below)
Use the built-in led in GPIO0 with a reverse signal.
Read the built-in temperature sensor using NTCSensor
Read built-in button states for simple reset or factory reset using GPIO2

⚠️ Fix before merge / help wanted

I didn't use i2c.ino and I would like to remove some compile warnings 💯
Different sensor instances

This is how looks like #0 are (RELAY 1 + RELAY 2) consumption, #1 Relay 1, and #2 Relay 2.

I would look at pzem module to know how to use a single instance.

mcspr · 2019-07-17T15:29:39Z

I didn't use i2c.ino and I would like to remove some compile warnings

Not seeing any warning with the latest commit 🤷‍♂
I would add 32 bit methods to the i2c.ino, to be consistent with other sensor implementations.

Different sensor instances

Already discussed in gitter. You can use just one instance + some internal configuration. If you want to keep combined reading, this can be an optional flag (e.g. specific mode for per-relay readings, same + combined and only combined). As already mentioned in the #1822, BMX280 is one example of magnitude config. PZEM004T takes this further, using magnitude list multiple times for the reasons described here.

IANAL, but I think the copyright comment should include you too? Maybe even with the comment below

tonilopezmr · 2019-07-18T09:38:52Z

First of all, thanks for your review and your time. I would like to make those changes by myself then you just need to review it. I'm going to continue this PR tomorrow afternoon. 🎉

tonilopezmr · 2019-07-20T16:42:56Z

Hi @mcspr I got an error trying to use vector to read and write the current and power values.

Exception Cause: 29  [StoreProhibited: A store referenced a page mapped with an attribute that does not permit stores]

0x40218f23: ADE7953Sensor::pre() at /espurna/code/espurna/config/../sensors/ADE7953Sensor.h:130
 (inlined by) ADE7953Sensor::pre() at /espurna/code/espurna/config/../sensors/ADE7953Sensor.h:114
0x40218ee0: ADE7953Sensor::pre() at /espurna/code/espurna/config/../sensors/ADE7953Sensor.h:100
0x4021232c: _sensorPre() at /espurna/code/espurna/sensor.ino:402
0x402378b6: wifi_get_opmode at ??:?
0x40216a1c: sensorLoop() at /espurna/code/espurna/sensor.ino:1502
0x402378b6: wifi_get_opmode at ??:?
0x40238713: wifi_station_get_connect_status at ??:?
0x402282b8: JustWifi::connected() at /espurna/code/.pio/libdeps/allterco-shelly25/JustWifi/src/JustWifi.cpp:796
0x4021a13c: ESP8266WiFiSTAClass::status() at /.platformio/packages/[email protected]/libraries/ESP8266WiFi/src/ESP8266WiFiSTA.cpp:453
0x40238713: wifi_station_get_connect_status at ??:?
0x402282b8: JustWifi::connected() at /espurna/code/.pio/libdeps/allterco-shelly25/JustWifi/src/JustWifi.cpp:796
0x4021a13c: ESP8266WiFiSTAClass::status() at /.platformio/packages/[email protected]/libraries/ESP8266WiFi/src/ESP8266WiFiSTA.cpp:453
0x40206fd0: wifiConnected() at /espurna/code/espurna/wifi.ino:569
0x40204e97: loop at /espurna/code/espurna/espurna.ino:254 (discriminator 2)
0x4022df70: loop_wrapper() at /.platformio/packages/[email protected]/cores/esp8266/core_esp8266_main.cpp:123
0x401006fc: cont_norm at cont.S.o:?
0x4010f000: ?? ??:0

I have let you the last code I have, but I have changed many times trying to fix it.

code/espurna/sensors/ADE7953Sensor.h

tonilopezmr · 2019-07-21T18:11:11Z

Thanks! I have applied your changes and now it seems it's working!

tonilopezmr · 2019-07-22T12:42:59Z

I don't know how to implement energy to save it, etc... I guess it should have something generic implemented to use, or every current metering sensor it's implementing the energy in its own way ??

mcspr · 2019-07-22T13:51:33Z

ref #1832 (comment), eneTotal<index>?
PZEM does use a separate key storage though, so it might need to use a generic one too.

Sidenote: how expensive it would be to use double instead of float? I have noticed that pzem api uses it too.

code/espurna/sensors/ADE7953Sensor.h

mcspr · 2019-07-22T14:04:21Z

code/espurna/sensors/ADE7953Sensor.h

+            _ready = true;
+        }
+
+        int Ade7953RegSize(uint16_t reg) {


This might as well be static, since we don't use any member access. And we already are inside a class, Ade7953 prefix is redundant.

I was somewhat confused by the sizing function, is this mentioned somewhere in the datasheet?

Kind-of related
If it is really how it works, I tried making it more compact (idk about what is more readable though...)

const int reg_size_new(const uint16_t reg) { const uint8_t mask = ((reg >> 8) & 0b1111); // edit: this was a redundant & 0b11111111, and is this even needed? if (!mask || (mask & 0b1100)) { return 1; } else if (mask & 0b0011) { return mask + 1; } return 0; }

You can try this with ~/.platformio/packages/toolchain-xtensa/bin/xtensa-lx106-elf-gdb (you'll need start building something with [email protected] platform first, it will move the old one to a separate dir)

(gdb) disassemble reg_size Dump of assembler code for function reg_size(unsigned short): 0x40201018 <+0>: extui a2, a2, 8, 4 0x4020101b <+3>: movi.n a3, 8 0x4020101d <+5>: bltu a3, a2, 0x40201069 <reg_size(unsigned short)+81> 0x40201020 <+8>: l32r a3, 0x40201014 0x40201023 <+11>: addx4 a2, a2, a3 0x40201026 <+14>: l32i.n a2, a2, 0 0x40201028 <+16>: jx a2 0x4020102b <+19>: and a6, a3, a0 0x4020102e <+22>: excw 0x40201031 <+25>: excw 0x40201034 <+28>: or a1, a0, a5 0x40201037 <+31>: and a5, a5, a4 0x4020103a <+34>: excw 0x4020103d <+37>: excw 0x40201040 <+40>: s32i.n a6, a0, 4 0x40201042 <+42>: excw 0x40201045 <+45>: excw 0x40201048 <+48>: excw 0x4020104b <+51>: and a6, a3, a4 0x4020104e <+54>: excw 0x40201051 <+57>: s8i a0, a6, 0 0x40201054 <+60>: andbc b0, b12, b0 0x40201057 <+63>: addi.n a2, a2, 1 0x40201059 <+65>: j 0x4020105e <reg_size(unsigned short)+70> 0x4020105c <+68>: movi.n a2, 0 0x4020105e <+70>: addi.n a2, a2, 1 0x40201060 <+72>: j 0x40201065 <reg_size(unsigned short)+77> 0x40201063 <+75>: movi.n a2, 0 0x40201065 <+77>: addi.n a2, a2, 1 0x40201067 <+79>: ret.n 0x40201069 <+81>: movi.n a2, 0 0x4020106b <+83>: ret.n (gdb) disassemble reg_size_new Dump of assembler code for function reg_size_new(unsigned short): 0x40202e9c <+0>: extui a3, a2, 8, 8 0x40202e9f <+3>: extui a5, a3, 0, 4 // edit: this is the "& 0b1111" aka bitfield extraction 0x40202ea2 <+6>: movi.n a2, 1 0x40202ea4 <+8>: beqz.n a5, 0x40202eb8 <reg_size_new(unsigned short)+28> 0x40202ea6 <+10>: movi.n a4, 12 0x40202ea8 <+12>: and a4, a3, a4 0x40202eab <+15>: bnez.n a4, 0x40202eb8 <reg_size_new(unsigned short)+28> 0x40202ead <+17>: add.n a5, a5, a2 0x40202eaf <+19>: extui a3, a3, 0, 2 0x40202eb2 <+22>: or a2, a4, a4 0x40202eb5 <+25>: movnez a2, a5, a3 0x40202eb8 <+28>: ret.n

At least what it seems like, that the code size goes down 77 bytes.
edit: ...it might also might not matter at all. i will try with full build later. current tests are with __attribute__((noinline)) an a small test project

I need to learn more about moving bytes, I'm not really sure what reg_size does, but it seems to be a very important function.

And just to be clear about this, what happens is:

>>> '{:#06b}'.format(0x31C >> 8) '0b0011' >>> '{:#06b}'.format(0x21C >> 8) '0b0010'

Then, when checking last 2 bits

>>> (0b0011 & 0b0011) + 1 4 >>> (0b0010 & 0b0011) + 1 3

So, the 0x3... prefix contains 4 bytes => 32 bits and 0x2... contains 3 bytes => 24 bits

code/espurna/sensors/ADE7953Sensor.h

code/espurna/config/hardware.h

tonilopezmr · 2019-07-22T17:07:18Z

Here you can play if you want with the new reg_size method (replacing it to the new one) and testing the declaration of struct, for me doesn't compile

mcspr · 2019-07-22T18:32:42Z

Should work?
Second reg_size function is on by default, struct moved up.
I think the type warning fix for addresses is wrong though (uint8_t for all), but it should go to the i2c.ino anyways later.
delay / nice_delay is more of a quirk of a platform. it is not technically always needed, but nice to have in case it does break.

tonilopezmr · 2019-07-22T18:34:27Z

Thanks!! @mcspr I'm going to test it right now. About energy?

is there any common implementation for all devices?
how I should implement it?
It saves energy consumption if turn off and on?

mcspr · 2019-07-22T18:53:11Z

_sensorEnergyTotal(index), _sensorEnergyTotal(index, value) ? see sensor.ino
It saves after reading N times, sensorLoop calls it every read to put it in persistent memory
(aka Rtcmem, does not erase between resets. Implemented as a pointer to that memory chunk and using it as basic struct)
Settings are pretty straightforward
Rtcmem I am not sure, keep it like this and only save index 0? Or, extend energy to energy0, energy1, etc. It needs to have known size on compilation

mcspr · 2019-08-12T16:56:51Z

@tonilopezmr any luck so far? is energy saving still a problem?

tonilopezmr · 2019-08-20T16:11:03Z

I sorry I was on holidays, yes, I need to implement it, also I need to fix a problem with led in GPIO0, it's not working, as you can see the configuration it seems right, but GPIO0 is not set as OUTPUT.

tonilopezmr · 2019-08-22T18:05:11Z

I thought _sensorEnergyTotal(index, value) exists, but I only see void _sensorEnergyTotal(double value) should I create it in sensor.ino adding an index ?

mcspr · 2019-08-23T09:30:29Z

Sure, that's what I was thinking.

And idk why I was always referring to the PZEM sensor, EmonADS1X15Sensor.h also supports multiple "ports" and is based on the EmonSensor / I2CSensor. It is analog converter, though... So, for example, rms value is manually calculated while ADE chip already does this for us.

--
As mentioned in gitter, I'd think we can somehow figure out a way to sync up readings with how fast ADE calculates data. What I was not sure about is how exactly make this happen, when documentation only mentioned interrupt pins and per-channel state registers as a method to receive state updates.

edit: e.g. zero line crossing cycles, depending on ac frequency (is that what they mean by half line cycles, on just one side?). and energy register, which could be avoided and approximated via power value, is expected to remain the same for some time:

The contents of the AENERGYA and AENERGYB registers are updated synchronous to the CYCEND flag. The AENERGYA and AENERGYB registers hold their current values until the end of the next line cycle period, when the contents are replaced with the new reading. If the read-with-reset bit (RSTREAD) in the LCYCMODE register (Address 0x004) is set, the contents of the AENERGYA and AENERGYB registers are cleared after a read and remain at 0 until the end of the next line cycle period.

--
It has the same problem with energy saving by the looks of it, since there's no mechanism to restore energy per port, and current API is misleading for single source devices (ref HLW, first argument is value and not idx).

…o support-shelly25

mcspr · 2019-08-24T20:07:35Z

Nice. BTW, while I was describing that energy accumulator, I missed that it needs to be explicitly enabled via ACCMODE, LCYCMODE and LINECYC registers. As I understood it, AENERGY... registers will contain per-cycle values and would be immediately reset to 0 after i2c read is done. Only after next cycle finishes ADE will update the register.
Have you tried reading those manually via cli commands or periodically as part of sensor read, maybe just for a test? Energy value could be more precise that way (depends on load though)

Calibration is also missing, that is something to add later. See A/B WGAIN, A/B IGAIN

Does it make sense to save the first "dev" at all? edit: Maybe it should be a sensor module option, combining all of energy values into one? Per-sensor or globally.

I'll clean up flow a bit, combining with other sensors to avoid having special conditional handling per sensor type. Maybe also finally fix reset timestamping...

tonilopezmr · 2019-08-26T09:49:00Z

I think for a first "dev" version it's fine, I have been testing this weekend and works fine, I would like to improve my knowledge about I2C and ADE, I would create an ISSUE to work on getting energy from ADE sensor, with this comment's help.

can you give me more information about reading those values using cli commands? do you mean espurna commands?

Thanks!

mcspr · 2019-08-26T21:24:58Z

Yes, terminal. Something to inspect sensor state, which in this case is to read configuration and value registers. i2c.read <bits> <addr> ?

edit: and write too, since energy registers need config

mcspr · 2019-08-26T23:25:21Z

code/espurna/sensor.ino

@@ -462,18 +462,37 @@ void _sensorResetTS() {
    #endif
 }

-double _sensorEnergyTotal() {
+double _sensorEnergyTotal(unsigned int index = -1) {


I missed this earlier... note the unsigned :) Fixed via #1875

tonilopezmr added 3 commits July 14, 2019 23:57

Add Shelly 2.5

1febe9b

Add ADE7953 sensor support

eae8899

Fix compilation

4c9ca84

tonilopezmr added 2 commits July 20, 2019 15:39

Use only one instance

10fdd27

WIP: Not working, I got StoreProhibited exception

e63c5e1

mcspr reviewed Jul 20, 2019

View reviewed changes

code/espurna/sensors/ADE7953Sensor.h Outdated Show resolved Hide resolved

mcspr reviewed Jul 20, 2019

View reviewed changes

code/espurna/sensors/ADE7953Sensor.h Outdated Show resolved Hide resolved

Fix exception

440a7cd

Resolve conflicts

e873af7

mcspr reviewed Jul 22, 2019

View reviewed changes

tonilopezmr added 2 commits July 22, 2019 18:40

Fix merge issues

1b49304

Apply request review changes

e3ea8f1

mcspr added 4 commits July 22, 2019 21:21

declare struct before using it

d460581

uninterruptible delay

9dbaeb3

try optimized reg_size

c20f190

fix types

3246390

tonilopezmr added 2 commits August 21, 2019 16:04

Remove gpio 0 input in analog sensor

1c3d68a

Add energy to ADE7953, it is not finished yet

f0bcaa1

tonilopezmr added 4 commits August 23, 2019 12:51

Merge branch 'dev' into support-shelly25

6f0a39d

Add new method to save energy with index

104a641

Merge branch 'support-shelly25' of github.com:tonilopezmr/espurna int…

08dadb9

…o support-shelly25

Finish energy for ADE7953

7df5817

tonilopezmr mentioned this pull request Aug 26, 2019

Improve sensor ADE7953 energy readings #1870

Open

mcspr merged commit 93c1dd6 into xoseperez:dev Aug 26, 2019

mcspr reviewed Aug 26, 2019

View reviewed changes

tonilopezmr deleted the support-shelly25 branch August 27, 2019 13:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Shelly 2.5 #1827

Support Shelly 2.5 #1827

tonilopezmr commented Jul 14, 2019 •

edited

Loading

mcspr commented Jul 17, 2019

tonilopezmr commented Jul 18, 2019

tonilopezmr commented Jul 20, 2019 •

edited

Loading

tonilopezmr commented Jul 21, 2019 •

edited

Loading

tonilopezmr commented Jul 22, 2019

mcspr commented Jul 22, 2019

mcspr Jul 22, 2019 •

edited

Loading

tonilopezmr Jul 22, 2019

mcspr Aug 23, 2019

tonilopezmr commented Jul 22, 2019

mcspr commented Jul 22, 2019

tonilopezmr commented Jul 22, 2019

mcspr commented Jul 22, 2019

mcspr commented Aug 12, 2019

tonilopezmr commented Aug 20, 2019

tonilopezmr commented Aug 22, 2019 •

edited

Loading

mcspr commented Aug 23, 2019 •

edited

Loading

mcspr commented Aug 24, 2019 •

edited

Loading

tonilopezmr commented Aug 26, 2019

mcspr commented Aug 26, 2019 •

edited

Loading

mcspr Aug 26, 2019

tonilopezmr Aug 27, 2019

Support Shelly 2.5 #1827

Support Shelly 2.5 #1827

Conversation

tonilopezmr commented Jul 14, 2019 • edited Loading

📌 References

💣 Issue

🎩 What is the goal?

📝 How is it being implemented?

⚠️ Fix before merge / help wanted

mcspr commented Jul 17, 2019

tonilopezmr commented Jul 18, 2019

tonilopezmr commented Jul 20, 2019 • edited Loading

tonilopezmr commented Jul 21, 2019 • edited Loading

tonilopezmr commented Jul 22, 2019

mcspr commented Jul 22, 2019

mcspr Jul 22, 2019 • edited Loading

Choose a reason for hiding this comment

tonilopezmr Jul 22, 2019

Choose a reason for hiding this comment

mcspr Aug 23, 2019

Choose a reason for hiding this comment

tonilopezmr commented Jul 22, 2019

mcspr commented Jul 22, 2019

tonilopezmr commented Jul 22, 2019

mcspr commented Jul 22, 2019

mcspr commented Aug 12, 2019

tonilopezmr commented Aug 20, 2019

tonilopezmr commented Aug 22, 2019 • edited Loading

mcspr commented Aug 23, 2019 • edited Loading

mcspr commented Aug 24, 2019 • edited Loading

tonilopezmr commented Aug 26, 2019

mcspr commented Aug 26, 2019 • edited Loading

mcspr Aug 26, 2019

Choose a reason for hiding this comment

tonilopezmr Aug 27, 2019

Choose a reason for hiding this comment

tonilopezmr commented Jul 14, 2019 •

edited

Loading

tonilopezmr commented Jul 20, 2019 •

edited

Loading

tonilopezmr commented Jul 21, 2019 •

edited

Loading

mcspr Jul 22, 2019 •

edited

Loading

tonilopezmr commented Aug 22, 2019 •

edited

Loading

mcspr commented Aug 23, 2019 •

edited

Loading

mcspr commented Aug 24, 2019 •

edited

Loading

mcspr commented Aug 26, 2019 •

edited

Loading