Add P155_Smartmeter by Brommander · Pull Request #5542 · letscontrolit/ESPEasy

Brommander · 2026-05-08T06:30:11Z

No description provided.

TD-er · 2026-05-08T08:21:47Z

+  :header: "Store", "Link"
+  :widths: 5, 40
+
+  "eBay","`TTL/UART IR Reading Head <https://www.ebay.de/itm/314015465828>`_"


Maybe you could take a picture of the device you have (don't use pictures from the ebay page), as the ebay link may not remain to work forever.

Should I remove the link and just use a description and the image? The link was just an example; devices from other manufacturers work as well.

I assume you have the product yourself?
So you can also include a photo of it in the docs as a picture tells more than a 1000 words :)
You can also add some schematic drawing of where wires should be connected to for example.
And to make it perfect, place it on some grid paper showing the dimensions, like mm grid paper.

ok good, i tried to show what it looks like when it's installed and what the device looks like on its own. The wiring is quite simple so i think a table should be enough to understand

TD-er · 2026-05-08T08:34:40Z

+  case PLUGIN_DEVICE_ADD:
+  {
+    Device[++deviceCount].Number = PLUGIN_ID_155;
+    Device[deviceCount].Type = DEVICE_TYPE_DUMMY;


dev.Type = DEVICE_TYPE_SERIAL;

Also please use auto& dev = Device[++deviceCount]; and then replace Device[deviceCount] with dev

TD-er · 2026-05-08T08:36:34Z

+The plugin uses Hardware Serial2 on fixed pins:
+
+* RX = GPIO 16
+* TX = GPIO 17


Ehh this should be using a configuration to select a port.
Please search the code for ESPEasySerial, as used in plugins like P052/P053/P054 etc.
Not all ESP boards have the same pins accessible.
For example on ESP32-classic exactly these 2 pins have issues as they can also be used for PSRAM. Even on a board without PSRAM these pins will show high frequency signals every now and then.
I noticed you are using ESPEasySerial to create the UART instance, but still using hard coded values.
When using the dev.type (in the PLUGIN_ADD) to be serial, you already have the selector present on the webpage.

Then you could add something like this:

case PLUGIN_WEBFORM_SHOW_SERIAL_PARAMS: { String options_baudrate[6]; for (int i = 0; i < 6; ++i) { options_baudrate[i] = String(p085_storageValueToBaudrate(i)); } const FormSelectorOptions selector(6, options_baudrate); selector.addFormSelector(F("Baud Rate"), P085_BAUDRATE_LABEL, P085_BAUDRATE); addUnit(F("baud")); addFormNumericBox(F("Modbus Address"), P085_DEV_ID_LABEL, P085_DEV_ID, 1, 247); break; }

to fine-tune the serial config (taken as example from P085)

TD-er · 2026-05-08T08:36:42Z

+
+    CONFIG_PORT = 5; // Serial2
+    CONFIG_PIN1 = 16;
+    CONFIG_PIN2 = 17;


These are macros, you should not assign fixed values here.

const int16_t serial_rx = CONFIG_PIN1; const int16_t serial_tx = CONFIG_PIN2; const ESPEasySerialPort port = static_cast<ESPEasySerialPort>(CONFIG_PORT);

TD-er · 2026-05-08T08:38:36Z

+  "11", "L2_A", "1-0:51.7.0", "A", "Current L2"
+  "12", "L3_A", "1-0:71.7.0", "A", "Current L3"
+
+\* SML-Auto delivers energy values in Wh. Enter ``VALUE/1000`` in the formula field to get kWh.


I think the formula is %value%/1000

TD-er · 2026-05-08T08:39:02Z

+.. note::
+
+   With **SML-Auto**, energy registers deliver the raw value in Wh (scaler is read
+   automatically from the telegram). Enter ``VALUE/1000`` in the ESPEasy formula


TD-er · 2026-05-08T08:39:36Z

+//
+// SML-Auto Funktionsweise:
+//   Nach der OBIS-Kennung enthält jeder SML-ListEntry folgende Felder,
+//   jeweils eingeleitet durch ein TL-Byte (Type-Length):


Please use English, also in comments

TD-er · 2026-05-08T08:41:35Z

+
+#include <ESPeasySerial.h>
+
+ESPeasySerial *P155_MySerial = nullptr;


Usually we do have a PluginStruct for plugin runtime data so we can have multiple instances of a plugin.
Also this makes sure no unused global variables are allocated in memory. (like nearly everything below this line upto line 206)

But we can change this later.

TD-er · 2026-05-08T09:12:17Z

+{
+  String p155_rxID = "x-x:x.x.x*x";
+  float value;
+  p155_dataStructD0(String xID, float xvalue)


By calling this function, you essentially copy the string.
Better to use p155_dataStructD0(const String& xID, float xvalue) as declaration of the constructor.
And since you don't actually do anything with the arguments, it is even better to have the constructor like this:

p155_dataStructD0(const String& xID, float xvalue) : p155_rxID(xID), value(xvalue) {}

TD-er · 2026-05-08T09:31:50Z

+      break;
+    }
+  }
+  addLogMove(LOG_LEVEL_DEBUG, log);


If you declare the int i outside the scope of the for loop, you will still know which i was handled.
So then you can have all the log related lines together and wrap them into #ifndef BUILD_NO_DEBUG and if (loglevelActiveFor(LOG_LEVEL_DEBUG))

TD-er · 2026-05-08T09:33:16Z

+{
+  String log = F("D0 Parse: ID=");
+  log += p155_rxID;
+  for (int i = 1; i < p155_outputOptionsAct; i++)


Why starting at 1 here? If you don't need that one, why have it declared in the p155_myDataD0 array?

TD-er · 2026-05-08T09:35:53Z

+      raw |= (int32_t)0xFFFFFF00;
+    else if (readBytes == 2 && (rawU & 0x8000))
+      raw |= (int32_t)0xFFFF0000;
+    lvalue = (float)raw;


When assigning an int to a float type, you don't need these C-style casts

TD-er · 2026-05-08T09:36:09Z

+  }
+  else // unsigned (typ 6) oder unbekannt
+  {
+    lvalue = (float)rawU;


TD-er · 2026-05-08T09:40:06Z

+
+  float scaledValue = (p155_scaler != 0)
+                          ? lvalue * powf(10.0f, (float)p155_scaler)
+                          : lvalue;


This scalar only seems to be set once in the code.
So why not calculate the scalar value there and have the scalar be either 1.0f or powf(10.0f, X) with X being the scalar value.

TD-er · 2026-05-08T09:41:55Z

+float p155_readVal(uint8_t query, unsigned int model)
+{
+  if (model == 0)
+    return p155_myDataD0[query].value;


There is no check for the range of the arrays

TD-er · 2026-05-08T09:44:43Z

+uint8_t p155_charsRead = 0;
+char p155_rxBuffer[P155_RX_BUFFER];
+char p155_ringBuffer[8];
+byte p155_rxOrbis[6]; //|1|0|2|8|0|255


Arrays are not initialized.

char p155_rxBuffer[P155_RX_BUFFER]{}; char p155_ringBuffer[8]{}; byte p155_rxOrbis[6]{}; //|1|0|2|8|0|255

Adding {} will initialize them with all zeroes.

TD-er · 2026-05-08T09:45:34Z

+int p155_anzBytes;
+int p155_posDataAct;
+int p155_registerAct;
+int p155_outputOptionsAct;


Int values also not initialized.

int p155_anzBytes{}; int p155_posDataAct{}; int p155_registerAct{}; int p155_outputOptionsAct{};

This will initialize them with zeroes.

TD-er · 2026-05-08T09:56:59Z

+  String logdata1 = F("D0: Data=");
+
+  unsigned long timeOut = millis() + 10;
+  while (P155_MySerial->available() && millis() < timeOut)


Please use functions like timePassedSince() so we can be sure those time calculations are done correct.
Otherwise you may get odd errors every 49.7 days :)

Also available() can be a rather expensive function, so maybe better to store it before making the while loop and if there is time left, you can do another check for available().

const unsigned long start = millis(); size_t available = P155_MySerial->available(); while (available && timePassedSince(start) < 10) { --available; if (available == 0) { available = P155_MySerial->available(); } ...

TD-er · 2026-05-08T09:59:09Z

+
+    if (c == '(')
+    {
+      p155_rxBuffer[p155_charsRead] = '\0';


There is no check for p155_charsRead < NR_ELEMENTS(p155_rxBuffer)
Should be done at the start of the while loop. (or at the end as you may set this value to 0 in this loop)

letscontrolit#1 Translate all comments to English letscontrolit#2 Use const reference and member initializer list in constructors letscontrolit#3 Explicit {} initialization for global arrays (clarity) letscontrolit#4 Explicit {} initialization for global int variables (clarity) letscontrolit#5 Use auto& dev and DEVICE_TYPE_SERIAL in PLUGIN_DEVICE_ADD letscontrolit#6 Use const variables for serial port/pins from CONFIG letscontrolit#7 Add bounds checks before array index access letscontrolit#8 Replace millis() arithmetic with timePassedSince() letscontrolit#9 Add bounds check at top of D0 read loop letscontrolit#11 Replace C-style casts with static_cast<> letscontrolit#12 Pre-calculate scale factor (powf) in state 32, not per value letscontrolit#13 Declare loop variable outside scope for use in debug log letscontrolit#14 Remove hardcoded serial pins — port now configured via UI

TD-er · 2026-05-13T14:21:41Z

Just as a casual remark...

This I really really like and appreciate!

I wish I was this structured myself.

TD-er · 2026-05-13T14:31:53Z

+      uint8_t outputOptions = (model == 1)   ? P155_NR_OUTPUT_OPTIONS_MODEL1
+                              : (model == 2) ? P155_NR_OUTPUT_OPTIONS_MODEL2
+                              : (model == 3) ? P155_NR_OUTPUT_OPTIONS_MODEL3
+                                             : P155_NR_OUTPUT_OPTIONS_MODEL0;


Now we're entering the nitpicking phase :)
Maybe it is also a good idea to think about a slightly better description of "MODEL1" ... "MODEL3" ?
And if you found one, then also those "magic numbers" like 0...3 could be changed into some #define ... to make it more readable.

While thinking about what to name the different models so that they would still make sense down the road, I decided to remove two legacy models that I wanted to keep for compatibility reasons with the ESPs currently in use. so it is more easy to name the two left models

TD-er · 2026-05-13T14:32:16Z

+    p155_outputOptionsAct = (P155_MODEL == 1)   ? P155_NR_OUTPUT_OPTIONS_MODEL1
+                            : (P155_MODEL == 2) ? P155_NR_OUTPUT_OPTIONS_MODEL2
+                            : (P155_MODEL == 3) ? P155_NR_OUTPUT_OPTIONS_MODEL3
+                                                : P155_NR_OUTPUT_OPTIONS_MODEL0;


TD-er · 2026-05-13T14:37:29Z

  if (nullptr == P155_MySerial)
  {
-    addLog(LOG_LEVEL_INFO, F("SML: handleSerialIn nullptr"));
+    addLog(LOG_LEVEL_INFO, F("SML: Serial nullptr"));


How often is this function called?
If there is an error somehow, the last thing you would like is getting flooded with logs.
It is called from the PLUGIN_TEN_PER_SECOND call, so you may want to add some limit on the number of logs.

Simple limiter could be:

static uint32_t lastLogTime{}; if (timePassedSince(lastLogTime) > 1000) { addLog.... lastLogTime = millis(); }

TD-er · 2026-05-13T14:43:07Z

+    // Ring buffer for SML start-sequence detection: 1B 1B 1B 1B 01 01 01 01
    for (int i = 7; i > 0; i--)
      p155_ringBuffer[i] = p155_ringBuffer[i - 1];
    p155_ringBuffer[0] = b;


Hmm this I may need to think about a bit more as it feels quite inefficient.
Just leave it for now as it will likely not be that much of a performance hit, but I guess it could be done more efficient, especially since the length is a power of 2 so no expensive modulo operator is required.

Maybe shifting the values in an int64... ? I can't tell whether this is an advantage or whether splitting it into two 32-bit operations is actually counterproductive.

static uint64_t p155_rxWindow = 0;
p155_rxWindow = (p155_rxWindow << 8) | b; // per Byte (one Operation instead of 8 writings in loop):

static constexpr uint64_t P155_SML_startseq = 0x1B1B1B1B01010101ULL;
if (p155_rxWindow == P155_SML_startseq)// Compare (one Operation instead memcmp with 8 Bytes):

Why do you need to shift the buffer?
If you only need to compare it with some value, you can also change the compare-array.
And using an uint64_t is also possible, however I would not make it static as that renders it impossible to have multiple instances of the same plugin.

TD-er · 2026-05-13T14:48:40Z

This can be made way simpler by just using a simple memcmp

constexpr uint8_t SML_startseq = { 0x01, 0x01, 0x01, 0x01, 0x1B, 0x1B, 0x1B, 0x1B ); if (memcmp(p155_ringBuffer, SML_startseq, NR_ELEMENTS(SML_startseq)) == 0)

TD-er · 2026-05-13T14:56:18Z

    lint32 = ((uint8_t)p155_rxBuffer[lLen] << 24) |
             ((uint8_t)p155_rxBuffer[lLen + 1] << 16) |
             ((uint8_t)p155_rxBuffer[lLen + 2] << 8) |
             (uint8_t)p155_rxBuffer[lLen + 3];


In these (and following), there is no check for (lLen + 3) < P155_RX_BUFFER

Since you are making similar calls several times, maybe a good idea to put this in a function?
bool p155_getIntFromRxBuffer(uint32_t index, int32_t& value)

And maybe have a simple uint32_t variant first, which can then be cast into a signed int or a simple check to see if it is over some value and then convert it. (depends on how the int type is formatted)

TD-er

.

letscontrolit#1 Better description of models letscontrolit#2 reduce models to 2 letscontrolit#3 Logging nullptr with timer letscontrolit#4 use memcmp instead of long if letscontrolit#5 better check of lenght and memory access with functions

tonhuisman · 2026-05-13T19:24:07Z

+Output
+~~~~~~~~~


If you have the title-type marker longer than the title, the RestructureText compiler will throw a warning about that. Best to keep them the correct length

tonhuisman · 2026-05-13T19:27:29Z

 Indicators (recommended settings)
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


TD-er · 2026-05-13T19:43:20Z

+    return false;
+  result = static_cast<int32_t>(raw);
+  if (numBytes == 1 && (raw & 0x80))
+    result |= (int32_t)0xFFFFFF00;


Hmm that's a rather complex way of doing 2-complement conversion, right?

int64_t tmp = raw; const uint32_t mask = (0x01 << (numBytes *8 -1) ); if (tmp >= mask) tmp -= mask; result = tmp;

Untested code, but I guess it would be something like this, right?

I guess we really should have a function for this in the ESPEasy math code as it is done quite often and when having a single function for it, it is easier to read what is done as the function name tells it.

tonhuisman · 2026-05-13T21:31:05Z

+  case PLUGIN_DEVICE_ADD:
+  {
+    auto &dev = Device[++deviceCount];
+    dev.Number = PLUGIN_ID_155;
+    dev.Type = DEVICE_TYPE_SERIAL; // enables serial port selector in UI
+    dev.VType = Sensor_VType::SENSOR_TYPE_QUAD;
+    dev.Ports = 0;
+    dev.PullUpOption = false;
+    dev.InverseLogicOption = false;
+    dev.FormulaOption = true;


Looking at the code in general, you haven't used Uncrustify for formatting the code. That's a suggested practice though, so we have most (new) sources formatted in a default format. (some sources don't get formatted for historic reasons and having too many changes)
The instructions on how to install & setup Uncrustify are included in the Development setup documentation for ESPEasy, the default formatting hotkey in VSCode is Alt-Shift-F (Windows) or Cmd-Shift-F (MacOS) 😉

Add P155_Smartmeter

c84c15e

TD-er reviewed May 8, 2026

View reviewed changes

Comment thread src/_P155_Smartmeter.ino Outdated

}

else // unsigned (typ 6) oder unbekannt

{

lvalue = (float)rawU;

Copy link
Copy Markdown

Member

TD-er May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

TD-er reviewed May 8, 2026

View reviewed changes

Brommander and others added 3 commits May 13, 2026 15:17

Update Doc PR#5542

e270aa1

Merge branch 'letscontrolit:mega' into SynchESPEasy

fff00c8

TD-er reviewed May 13, 2026

View reviewed changes

Changes for letscontrolit#5542

8d6afa5

letscontrolit#1 Better description of models letscontrolit#2 reduce models to 2 letscontrolit#3 Logging nullptr with timer letscontrolit#4 use memcmp instead of long if letscontrolit#5 better check of lenght and memory access with functions

tonhuisman reviewed May 13, 2026

View reviewed changes

TD-er reviewed May 13, 2026

View reviewed changes

tonhuisman reviewed May 13, 2026

View reviewed changes


		#include <ESPeasySerial.h>

		ESPeasySerial *P155_MySerial = nullptr;

		Output
		~~~~~~~~~

Conversation

Brommander commented May 8, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TD-er May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TD-er May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TD-er May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TD-er commented May 13, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TD-er May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TD-er left a comment

Choose a reason for hiding this comment

Uh oh!

TD-er May 8, 2026 •

edited

Loading

TD-er May 8, 2026 •

edited

Loading

TD-er May 8, 2026 •

edited

Loading

TD-er May 13, 2026 •

edited

Loading