Adapt library to Homie 3.0.0 #550

bodiroga · 2018-11-05T12:34:55Z

Here you have my proposal to adapt the library to the newest Homie convention version 👍 I have tested the changes with a NodeMCU board and the provided examples compile successfully! Anyway, review the changes carefully, please, I'm not a C++ guy.

Some things that could be added in a future PR:

Add a new method to name each node array element individually (right now, the "node name" is the same as the "node id" -> strip_1, strip_2, strip_3 and strip_4 in the "LedStrip.ino").
Rework the "node" and "property" creation. For the "property" attributes, instead of providing all fields in the "advertise" function as parameters, add individual functions and chain them. For example: myNode.advertise("temperature").withName("Temperature").withDatatype("int").withUnit("ºC").withFormat("-5:50").settable(myFunction). Even if it is more verbose, IMO it is more readable and users don't have to remember the order of the parameters. Also, it makes easier to add more fields, because we don't have to overload the function signature.

Many thanks for all your amazing work and don't hesitate to criticize my code, feel free 😉

Best regards,

Aitor

PS: I have left all the individual commits in case you want to review them one by one, but I can squash them into a single one for the final merge.
PS2: related issues: #549, #467, #464

* update file permissions * Update CI Build

… +Send when online only (#512) * Add option for ota_updater script to connect to TLS-protected brokers * Fix ota_updater not working when device is in deep sleep mode

bodiroga · 2018-11-13T07:37:48Z

Hi @marvinroger and @timpur!

I'm going to add the "$retained" property attribute to conform to Homie 3.0.1 and I see this as the perfect opportunity to refactor the "advertise" function.

Do you agree that the best option is to define the advertise function with the only required attribute (id) and add the optional ones (name, unit, format, datatype, retained and settable) following a "builder" pattern? We actually do that with the settable attribute, so it's natural to extend that principle to the rest non-mandatory attributes. What's your favorite name? ".withName", ".name", ".setName",...?

Many thanks for you help and looking forward to hearing from you guys!

Aitor

bodiroga · 2018-11-14T13:30:37Z

Done, Homie 3.0.1 fully supported!

timpur · 2018-11-14T21:14:20Z

sorry need to still have a proper review ....

src/HomieNode.cpp

examples/LedStrip/LedStrip.ino

davidgraeff · 2018-11-16T15:50:44Z

src/HomieNode.cpp

 }

 PropertyInterface& PropertyInterface::setProperty(Property* property) {
  _property = property;
  return *this;
 }

-HomieNode::HomieNode(const char* id, const char* type, const NodeInputHandler& inputHandler)
+HomieNode::HomieNode(const char* id, const char* name, const char* type, bool range, uint16_t lower, uint16_t upper, const NodeInputHandler& inputHandler)


In all your examples the "id" is the same as "type".
"type" is actually optional, at least it doesn't say anything about the node in the controller perspective.
Could you maybe remove it from the constructor and make it a chainable setter instead?

Make all the array specific parameters a dedicated setter as well (setArray(uint16_t lower, uint16_t upper)). That way if arrays change at some point, we don't need to change the constructor.

"type" is actually optional, at least it doesn't say anything about the node in the controller perspective.

Type is a required attribute for Nodes in Homie 3.0.1, see here: https://homieiot.github.io/spec-core-v3_0_1/#node-attributes. That's why I have added it to the class constructor. Perhaps we can remove it for Homie 4.0, but that discussion doesn't belong here 😉

Make all the array specific parameters a dedicated setter as well (setArray(uint16_t lower, uint16_t upper)). That way if arrays change at some point, we don't need to change the constructor.

I didn't want to change that part of the code too much, because it was already implemented, just the minimum necessary changes. What do you think @marvinroger?

davidgraeff · 2018-11-16T15:55:59Z

src/SendingPromise.cpp

@@ -50,16 +50,16 @@ uint16_t SendingPromise::send(const String& value) {
  strcat(topic, Interface::get().getConfig().get().deviceId);
  strcat_P(topic, PSTR("/"));
  strcat(topic, _node->getId());
-  strcat_P(topic, PSTR("/"));
-  strcat(topic, _property->c_str());
-
  if (_range.isRange) {
    char rangeStr[5 + 1];  // max 65536
    itoa(_range.index, rangeStr, 10);
    strcat_P(topic, PSTR("_"));
    strcat(topic, rangeStr);
  }


Ranges are not part of Homie 3?

Ranges are how Node arrays are called in homie-esp8266, it's just an internal attribute name.

Be aware that "SendingPromise" is part of the Homie object, not of the Node. So if one Node sets the range it is used for other Nodes, too.
So Homierange needs to be reset after sending.

(TBH: I don't understad why it worked in V2?!)

@euphi
How can we do that ?

@jeanvdr :
Just add after line 57 (strcat(topic, rangeStr);):

_range.isRange = false; _range.index = 0;

It is a quick workaround. I think the range-mechanism has to be rewritten.

@euphi
Thanks

davidgraeff · 2018-11-16T15:56:42Z

examples/GlobalInputHandler/GlobalInputHandler.ino


-bool globalInputHandler(const HomieNode& node, const String& property, const HomieRange& range, const String& value) {
+bool globalInputHandler(const HomieNode& node, const HomieRange& range, const String& property, const String& value) {


Ranges are not part of Homie 3?

Ranges are how Node arrays are called in homie-esp8266, it's just an internal attribute name.

jmvaz · 2018-11-29T10:42:26Z

Hi! When PR will be merged?

ThomDietrich · 2018-12-20T16:27:48Z

@marvinroger any chance you'll find time to look into this one?

@bodiroga would you be interested to invest more time as a maintainer of this project?

davidgraeff · 2018-12-20T16:47:59Z

I just created arendst/Tasmota#4696. Would be nice to have all esp DIY firmwares out there supported. MySensors is also interested.

bodiroga · 2018-12-21T08:25:31Z

@bodiroga would you be interested to invest more time as a maintainer of this project?

My C++ knowledge is very limited to maintain a project like this, but I could help keeping the library up to date with the Homie convention 👍

GideonLeGrange · 2018-12-25T16:11:28Z

I wish we could get this merged and released 😕

I'm running my fork of @bodiroga 's develop-v3 branch and it's fine. ESPs running this code integrates easily into Openhab 2.4.

jmvaz · 2018-12-25T16:20:25Z

Come on guys! A little Christmas gift :)

manswiss · 2019-01-03T18:46:29Z

Please merge it. I usw actualy this Form and work perfect.
For OpenHAB 2.4 is Homie 3.0 nessesaire.
Thanks.

euphi · 2019-01-08T15:16:03Z

src/HomieNode.cpp

+      return Interface::get().getSendingPromise().setNode(*this).setProperty(property).setQos(1).setRetained(true);
+    else
+      return Interface::get().getSendingPromise().setNode(*this).setProperty(property).setQos(1);
+  }
 }


Here a reference to SendingPromise MUST be returned. For now, this result in a crash if you want to set a Property that has not been configured.

Solutions:

Change signature of HomieNode::setProperty to SendingPromise *HomieNode::setProperty(...) and return NULL --> caller can check for NULL

Create error message and abort() --> Homie-ESP8266 enforces that you can only send to advertised properties

Return a generic SendingPromise& to be able to send on arbitrary topics (old behaviour. Maybe that is what you wanted to do and just messed with the nested ifs?

I suggest to stick to the Homie-2.0 behaviour for now (third solution)

Just change the line 81 und 82 to one line:

if (iProperty && iProperty->isRetained())

euphi · 2019-01-08T19:38:19Z

src/HomieNode.hpp

+  void setDatatype(const char* datatype) { _datatype = datatype; }
+  void setFormat(const char* format) { _format = format; }
+  void setRetained(const bool retained = true) { _retained = retained; }
+


change to String (see detailed comment below)

euphi · 2019-01-08T19:41:24Z

src/HomieNode.hpp

+  const char* _name;
+  const char* _unit;
+  const char* _datatype;
+  const char* _format;


these should be stored as String, so it is automatically copied.

Of course most of the Strings that are set here a constants and thus in global memory.
However, it is possible to receive dynamically created strings here and these could be on stack memory. For example, the allowed values for an ENUM could be generated dynamically.

gorec2005 · 2019-01-14T11:36:01Z

@bodiroga Thank for you work - it's work, but ota_update don't :-(

mjcumming · 2019-01-17T21:04:33Z

Is it possible to do a pull on this PR? How?

dikodahan · 2019-01-18T18:00:58Z

+1 on having this released! Even if it will only have partial functionality (like OTA not working yet in initial release).
On the same subject of updating this to Homie 3, the current Homie 2 implementation has a dependency on ArduinoJson 5.x while 6.x has been released for a while now. Would be worth to update that support as well along the way, though back to my first point, could be a later release not to delay the initial support release.

Thank you all for this work, Amazing stuff!!!

euphi · 2019-01-18T20:02:26Z

@mjcumming You can pull the fork from bodiroga and chekout develop-v3

git clone https://github.com/bodiroga/homie-esp8266
cd homie-esp8266
git checkout develop-v3

With platformio, you can change your lib_deps from "Homie" to "https://github.com/bodiroga/homie-esp8266#develop-v3".

jeanvdr · 2019-01-19T14:02:46Z

@bodiroga, gorec2005, dikodahan
for ota_update to work, since topic $online has disappeared from homie v3.0.0, you have to change to 3 lines:
line 16 : client.subscribe("{base_topic}{device_id}/$online".format(**userdata)) > client.subscribe("{base_topic}{device_id}/$state".format(**userdata))
line 69: elif msg.topic.endswith('$online'): > elif msg.topic.endswith('$state'):
line 70: if msg.payload == 'false': > if msg.payload != 'ready':

That's it

mjcumming · 2019-01-20T18:38:40Z

@euphi thank you for the instructions!

euphi · 2019-01-20T19:56:44Z

It seems this branch sometimes looses events:

Triggering WIFI_DISCONNECTED event...
↕ Attempting to connect to Wi-Fi...
〽 Sending statistics...
  • Wi-Fi signal quality: 100%
  • Uptime: 1488s
✔ Wi-Fi connected, IP: 192.168.0.145
Triggering WIFI_CONNECTED event...
↕ Attempting to connect to MQTT...
〽 Sending statistics...
  • Wi-Fi signal quality: 8%
  • Uptime: 1548s
〽 Sending statistics...
  • Wi-Fi signal quality: 8%
  • Uptime: 1608s

--> MQTT Connected event is missing, although the homie-esp8266 client reconnected to MQTT. The LED is still blinking quickly. However, the client receives set-commands and handles them correctly, so this is a minor issue.

davidgraeff · 2019-01-20T20:09:55Z

Actually. The over-use of the Arduino String class and std::function will fragment the heap memory very fast and I assume this firmware is just crashing from time to time. The esp is quite fast with rebooting and reconnecting so that may not have been noticed yet.

euphi · 2019-01-20T20:50:44Z

@davidgraeff RTOS seems to be quite robust.

When using many debug outputs with format strings, I have some problems that outgoing MQTT messages are lost, but no crashes/resets.

All of my homie devices run very stable (uptime is months and I never saw any reset)

Exception:

one device using the WS2812FX library resets very often (5-30 minutes). I investigated a lot and haven't found any issue yet, so I more and more think this is a power supply issue.
some devices with bad Wifi connections start to reconnect/loose connection over and over, once they lost there wifi-connection for first time.
I think that maybe the initial amount of MQTT traffic is too much for a slow/disturbed Wifi connection. However, this is not a reset.
one device has I2C problems every now and then. For now, I blame the I2C device (PCF8575) for that.

euphi · 2019-01-22T00:01:10Z

Initial development for Homie Convention V3.0.x by @bodiroga

timpur and others added 27 commits June 2, 2018 13:10

update file permissions (#525)

158c645

* update file permissions * Update CI Build

Add option for ota_updater script to connect to TLS-protected brokers…

6d6bb54

… +Send when online only (#512) * Add option for ota_updater script to connect to TLS-protected brokers * Fix ota_updater not working when device is in deep sleep mode

Fix CI PK algorithm used

4971722

Add 'name' field to HomieNode

54a8ff2

Update Homie version (3.0.0)

edeec3b

Add 'init' state to advertise progress

387faf1

Add 'stats' state to advertise progress

c1ccd53

Add 'ready' state to advertise progress

d993351

Add 'PUB_NAME' state to advertise progress

8dd7bd3

Add 'name', 'unit', 'datatype' and 'format' fields to 'Property' class

679f480

Initialize 'name', 'unit', 'datatype' and 'format' to null

2291552

Set global step to 'PUB_INIT' on disconnect

de1b527

Publish properties attributes on setup

69796c3

Fix 'NULL' spelling

12755a6

Fix typo

ea568b0

Fix typos

f09d45d

Remove property attribute setters and overload 'advertise' function

ebee2a9

Rework the property attribute announcement mechanism

6c087e4

Remove ':settable' indicator from properties advertisement

f55450d

Change 'will' topic and message to '$state'->'lost'

9ba37a9

Set '$state' attribute to "sleeping'

5f1b5cf

Update all examples

28660ea

Update library version

16e8168

Implement node arrays functionality

2adbe11

Adapt examples

769fa72

Update README file

de21d23

Fix cpplint complains

d1896b0

bodiroga added 2 commits November 13, 2018 16:27

Introduce individual set functions for optional attributes of properties

61aef24

Update examples to the new advertise syntax

2bd5eef

Add $retained attribute to properties (Homie 3.0.1) and button example

f535dcd

davidgraeff reviewed Nov 16, 2018

View reviewed changes

Address review comments

6079dcb

euphi reviewed Jan 8, 2019

View reviewed changes

euphi changed the base branch from master to develop-v3 January 21, 2019 23:58

Merge branch 'develop-v3' into develop-v3

f91f1d4

euphi merged commit e22aec6 into homieiot:develop-v3 Jan 22, 2019

Misiu mentioned this pull request Feb 5, 2019

openHAB auto-discovery xoseperez/espurna#1442

Open

bodiroga deleted the develop-v3 branch April 16, 2019 12:50


		bool globalInputHandler(const HomieNode& node, const String& property, const HomieRange& range, const String& value) {
		bool globalInputHandler(const HomieNode& node, const HomieRange& range, const String& property, const String& value) {

Adapt library to Homie 3.0.0 #550

Adapt library to Homie 3.0.0 #550

Conversation

bodiroga commented Nov 5, 2018 • edited Loading

bodiroga commented Nov 13, 2018

bodiroga commented Nov 14, 2018

timpur commented Nov 14, 2018

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

bodiroga Nov 17, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

euphi Jan 20, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bodiroga Nov 17, 2018 • edited Loading

Choose a reason for hiding this comment

jmvaz commented Nov 29, 2018

ThomDietrich commented Dec 20, 2018

davidgraeff commented Dec 20, 2018

bodiroga commented Dec 21, 2018

GideonLeGrange commented Dec 25, 2018

jmvaz commented Dec 25, 2018

manswiss commented Jan 3, 2019

euphi Jan 8, 2019 • edited Loading

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

gorec2005 commented Jan 14, 2019

mjcumming commented Jan 17, 2019

dikodahan commented Jan 18, 2019

euphi commented Jan 18, 2019 • edited Loading

jeanvdr commented Jan 19, 2019

mjcumming commented Jan 20, 2019

euphi commented Jan 20, 2019

davidgraeff commented Jan 20, 2019

euphi commented Jan 20, 2019

euphi commented Jan 22, 2019

bodiroga commented Nov 5, 2018 •

edited

Loading

bodiroga Nov 17, 2018 •

edited

Loading

euphi Jan 20, 2019 •

edited

Loading

bodiroga Nov 17, 2018 •

edited

Loading

euphi Jan 8, 2019 •

edited

Loading

euphi commented Jan 18, 2019 •

edited

Loading