iec62056: Initial include of iec62056 module #19168

bergzand · 2023-01-18T11:56:50Z

Contribution description

This PR adds a new iec62056 module for reading and parsing smart energy meter data. Main focus at this point is the iec62056-21 sub-specification for local exchange of smart meter data.

The module contributed here can be used as basis for reading DSMR compatible meters exposing a P1 port (IEC62056-21 mode D). I think it can also be used to parse messages from German meters with infrared communication (Mode A through C)

Testing procedure

A test application is provided. Test data is gathered from the local smart meter available here.

Issues/PRs references

None

riot-ci · 2023-02-16T12:59:27Z

Murdock results

❌ FAILED

78a80f9 iec62056: Add test application

Success	Failures	Total	Runtime
4222	0	6879	06m:52s

Artifacts

benpicco

I took a quick look

benpicco · 2023-02-16T12:53:14Z

sys/iec62056/iec62056_21.c

+#define ENABLE_DEBUG 0
+#include "debug.h"
+
+#define MIN(a, b)   (a > b ? b : a)


Suggested change

#define MIN(a, b) (a > b ? b : a)

#include "macros/utils.h"

I swear that didn't exist yet back when I wrote this code :)

benpicco · 2023-02-16T12:55:53Z

sys/iec62056/iec62056_21.c

+    time->tm_sec = scn_u32_dec(dataset->value + 10, 2);
+    time->tm_isdst = dataset->value[12] == 'S';
+
+    mktime(time); /* Normalize just in case */


Suggested change

mktime(time); /* Normalize just in case */

rtc_tm_normalize(time); /* Normalize just in case */

would be a bit cheaper - but I'm not sure if we should really normalize the data here, I wouldn't expect the device to output 'unnormalized' data and normalizing invalid data will just create a very hard to find error. (It's easier to recognize garbage timestamps than timestamps that are somewhat off)

Yeah, I see what you mean. To be honest I really don't trust smart meters to always output valid timestamp data, hence my idea to normalize it. But maybe this is a case of garbage in -> garbage out.

benpicco · 2023-02-16T12:59:06Z

sys/iec62056/iec62056_obis.c

+#include "iec62056.h"
+#include "iec62056/obis.h"
+
+#define MAX(a, b)   (a > b ? a : b)


Suggested change

#define MAX(a, b) (a > b ? a : b)

#include "macros/utils.h"```

benpicco · 2023-02-16T13:00:05Z

sys/iec62056/iec62056_obis.c

+    int res = 0;
+    char previous_separator = '\0';
+
+    memset(obis, 255, sizeof(iec62056_obis_t));


Why 255 instead of 0?

According to iec62056, the value 0 in an obis is valid data. 255 as value is either reserved, manufacturer specific or signals that the field is unused (in the case of value group F)

benpicco · 2023-02-16T13:00:36Z

sys/include/iec62056.h

+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#ifdef __cplusplus
+}
+#endif


If this file is empty, why have it?

It was useful once, but now it is just there for the doxygen grouping. I'll rework it a bit and remove the file

replaced with a doc.txt in sys/iec62056 for some documentation and a doxygen group.

benpicco · 2023-02-16T13:01:34Z

sys/include/iec62056/21.h

+ * @returns negative when parsing failed, zero on success
+ */
+int iec62056_21_dataset_parse_register_to_decfrac(const iec62056_21_dataset_t *dataset,
+                                                  int64_t *mantissa, int64_t *scaling);


Do we really get 128 bit values from this?

From IEC62056-21, on the length of the data:

Value: 32 printable characters maximum with the exception of (, ), *, / and !. For decimal values, only points (not commas) shall be used and shall be counted as characters.

Values can be large here, but I don't know if those appear in the wild, I'm severely lacking real world test cases to be honest.

benpicco · 2023-02-16T13:02:47Z

sys/include/iec62056/obis.h

+static inline iec62056_obis_t iec62056_obis_init(uint8_t a, uint8_t b, uint8_t c, uint8_t d,
+                                                 uint8_t e,
+                                                 uint8_t f)
+{
+    return (iec62056_obis_t){ a, b, c, d, e, f };
+}


This function seems a bit superfluous

It's mostly a convenience function for initializing the object. I clarified that in the documentation. Let me know if the function should be removed.

benpicco · 2023-02-16T13:05:49Z

sys/iec62056/iec62056_21.c

+        return res;
+    }
+
+    *value = (double)mantissa * pow(10, scaling);


Suggested change

*value = (double)mantissa * pow(10, scaling);

*value = (float)mantissa * powf(10, scaling);

is single precision not enough?
A iec62056_21_dataset_parse_register_to_float() might be handy if we don't want to pull in soft-float on e.g. Cortex-M4F.

Added a iec62056_21_dataset_parse_register_to_float().

benpicco · 2023-02-16T13:19:10Z

sys/iec62056/iec62056_21.c

+int iec62056_21_dataset_parse_register_to_decfrac(const iec62056_21_dataset_t *dataset,
+                                                  int64_t *mantissa, int64_t *scaling)
+{
+    /* parse a string with format '001234.567*kWh' to components */


This could be made a bit more reusable by moving the number parsing part (01234.567) to the fmt module - or just by using sscanf() for the number

benpicco · 2023-02-16T13:21:29Z

sys/iec62056/iec62056_21.c

+    IEC62056_21_STATE_OBIS,
+    IEC62056_21_STATE_NEXT,
+    IEC62056_21_STATE_COSEM_OBJECT,
+    IEC62056_21_STATE_LASTLINE,


Some comments would be nice

basilfx · 2023-10-27T12:22:02Z

sys/iec62056/iec62056_21.c

+#include <time.h>
+#include <math.h>
+
+#include "checksum/ucrc16.h"


Does not seem to be used.

basilfx · 2023-10-27T12:24:13Z

sys/iec62056/iec62056_21.c

+                                 const char *telegram, size_t len, bool *skip)
+{
+    /* find the end of the obis ID */
+    const char *obj = memchr(telegram, '(',  len);


Suggested change

const char *obj = memchr(telegram, '(', len);

const char *obj = memchr(telegram, '(', len);

basilfx · 2023-10-27T12:24:37Z

sys/iec62056/iec62056_obis.c

+bool iec62056_obis_equal(const iec62056_obis_t *first, const iec62056_obis_t *second,
+                         bool include_channel)
+{
+    return (first->a == second->a)  &&


Suggested change

return (first->a == second->a) &&

return (first->a == second->a) &&

basilfx · 2023-10-27T12:24:49Z

sys/include/iec62056/21.h

+ *
+ * IEC 62056-21 functions for direct local data exchange of electricity metering.
+ *
+ * Function implement protocol  telegram parsing.


Suggested change

* Function implement protocol telegram parsing.

* Function implement protocol telegram parsing.

basilfx · 2023-10-27T12:25:11Z

sys/include/iec62056/obis.h

+    uint8_t c;  /**< Group C: Abstract or physical data item identifier */
+    uint8_t d;  /**< Group D: Type or result of processing of the physical quantity */
+    uint8_t e;  /**< Group E: Further processing or classification */
+    uint8_t f;  /**< Group F: Identifies historical values  of data */


Suggested change

uint8_t f; /**< Group F: Identifies historical values of data */

uint8_t f; /**< Group F: Identifies historical values of data */

basilfx · 2023-10-27T12:27:57Z

sys/include/iec62056/obis.h

+ * @brief OBIS identifier struct
+ */
+typedef struct {
+    uint8_t a;  /**< Group A: media/energy type */


Suggested change

uint8_t a; /**< Group A: media/energy type */

uint8_t a; /**< Group A: Media/energy type */

Or, remove capital from the other group names.

bergzand added the Type: new feature The issue requests / The PR implemements a new feature for RIOT label Jan 18, 2023

bergzand requested review from miri64, smlng, leandrolanzieri, aabadie, MichelRottleuthner and fjmolinas as code owners January 18, 2023 11:56

github-actions bot added Area: doc Area: Documentation Area: sys Area: System Area: tests Area: tests and testing framework labels Jan 18, 2023

bergzand force-pushed the pr/iec62056_21 branch 5 times, most recently from 9005a2b to 78a80f9 Compare January 19, 2023 14:53

benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Feb 16, 2023

benpicco reviewed Feb 16, 2023

View reviewed changes

bergzand added 2 commits August 11, 2023 13:12

iec62056: Add initial module

7ef6878

iec62056: Add test application

ec9f743

bergzand force-pushed the pr/iec62056_21 branch from 78a80f9 to ec9f743 Compare August 11, 2023 11:13

bergzand removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Aug 11, 2023

bergzand added 3 commits August 11, 2023 13:44

fixup! iec62056: Add initial module

d275e13

fixup! fixup! iec62056: Add initial module

57827ec

fixup! fixup! fixup! iec62056: Add initial module

b996fdd

basilfx reviewed Oct 27, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

iec62056: Initial include of iec62056 module #19168

iec62056: Initial include of iec62056 module #19168

bergzand commented Jan 18, 2023

riot-ci commented Feb 16, 2023

benpicco left a comment

benpicco Feb 16, 2023

bergzand Aug 11, 2023

benpicco Feb 16, 2023

bergzand Aug 11, 2023

benpicco Feb 16, 2023

benpicco Feb 16, 2023

bergzand Aug 11, 2023

benpicco Feb 16, 2023

bergzand Aug 11, 2023

bergzand Aug 11, 2023

benpicco Feb 16, 2023

bergzand Aug 11, 2023

benpicco Feb 16, 2023

bergzand Aug 11, 2023

benpicco Feb 16, 2023

bergzand Aug 11, 2023

benpicco Feb 16, 2023

benpicco Feb 16, 2023

basilfx Oct 27, 2023

basilfx Oct 27, 2023

basilfx Oct 27, 2023

basilfx Oct 27, 2023

basilfx Oct 27, 2023

basilfx Oct 27, 2023

	mktime(time); /* Normalize just in case */
	rtc_tm_normalize(time); /* Normalize just in case */

	#define MAX(a, b) (a > b ? a : b)
	#include "macros/utils.h"```

	value = (double)mantissa pow(10, scaling);
	value = (float)mantissa powf(10, scaling);

	const char *obj = memchr(telegram, '(', len);
	const char *obj = memchr(telegram, '(', len);

	return (first->a == second->a) &&
	return (first->a == second->a) &&

	* Function implement protocol telegram parsing.
	* Function implement protocol telegram parsing.

	uint8_t f; /*< Group F: Identifies historical values of data /
	uint8_t f; /*< Group F: Identifies historical values of data /

	uint8_t a; /*< Group A: media/energy type /
	uint8_t a; /*< Group A: Media/energy type /

iec62056: Initial include of iec62056 module #19168

Are you sure you want to change the base?

iec62056: Initial include of iec62056 module #19168

Conversation

bergzand commented Jan 18, 2023

Contribution description

Testing procedure

Issues/PRs references

riot-ci commented Feb 16, 2023

Murdock results

Artifacts

benpicco left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment