homieiot / homie-esp8266

💡 ESP8266 framework for Homie, a lightweight MQTT convention for the IoT
http://homieiot.github.io/homie-esp8266
MIT License
1.36k stars 308 forks source link

Homie 2.0: Create an option for boot normal mode #354

Closed cecilios closed 5 years ago

cecilios commented 7 years ago

I am building an IoT irrigation controller for the garden, based on MQTT. It is, basically, an standalone device that should work by itself when the network is not available. Of course the design expects a network and all interaction with the irrigation controller (i.e. programming irrigations, setting times, etc.) is done via MQTT and needs network connection. But it the network fails, the controller must continue doing its job and all scheduled irrigations should be done.

I have in operation in my garden an initial version of this irrigation controller and I was trying to implement OTA update when I found your Hoime project. By the way, Homie is an excellent project!! Congratulations!. As Homie fits perfectly for all other devices I have to develop my plan is to use Homie for them. And so, I decided to study the convenience of preparing a version of my irrigation controller based on Homie. It will cost me more or less the same than developing my own OTA update code. Therefore as my controller is a class, I derived it from HomieNode and removed some unnecesary services (basically dealing with network and MQTT connections). In a couple of days the irrigation controller was operational as a Homie node.

Now I am evaluating if definitelly the irrigation controller device should follow this approach (the irrigation controller as a Homie node) or better continue with my previous approach (code independent from Homie, but MQTT messages compatible). The main drawback for moving the irrigation controller to Homie is that in Homie the nodes in a device do not receive service (their loop method is not invoked) when the network connection is lost. And this is a serious problem for me, as LAN connectivity can be lost by many reasons but the important thing is that irrigations must continue according to the scheduled programs. Network connectivity is just a commodity for supervising the device operation (feedback received via MQTT) or for changing operational parameters via MQTT (setting irrigation programs, times, etc.). Not having connectivity it is not a serious problem for stopping irrigations.

The Standalone boot mode is not of use in this case, as it assumes no network connection.

What I really need is that in normal boot mode, in case of no connectivity, the Nodes::loop() method should be invoked so that the irrigation controller can do its job (but as there is no connectivity it will not send MQTT feedback to the domotic controller).

This would require a modification in BootNormal.cpp, just to remove the loop:

 for (HomieNode* iNode : HomieNode::nodes) {
      iNode->loop();
    }

from inside the if statement:

 if (Interface::get().connected) {
  }

But as this would create incompatibilities with existing software using Homie, I would suggest to create either a new boot node (NORMAL_STANDALONE) or an option for boot normal method. Would that be posible?

Thank you.

jamesgol commented 7 years ago

This is similar to a need of mine and I haven't looked at the code yet, but I wonder if yet another boot mode is necessary. Maybe adding a config flag for something like 'requireNetwork' which defaults to true and provides existing behavior, but if set to false will run the loops but still search for network/mqtt broker.

cecilios commented 7 years ago

@james gol Yes, an option would be enough. But when that option is set, your code must not send mqtt messages when no LAN connection, to avoid creating a long queue of pending messages that could take up all the RAM.

As I posted this suggestion time ago and there have not been any feedback about rejection/acceptance of this, I just forked the Homie project and did this modification as well as other I needed, such as gettable properties, range nodes and finding mqtt broker IP via mdsn. Perhaps in future, if Marvin is interested, I could contribute back a PR with some or all of my modifications.

jamesgol commented 7 years ago

Good to know, when I get home I'll check your fork out. My use case is similar to yours, I've got a couple of refrigerators I'm controlling the temperature on. It's nice to have network to adjust the temp or log the data but I want them to control at the last set temp regardless of MQTT or network availability.

cecilios commented 7 years ago

@jamesgol Sorry but I didn't publish the fork. It was not my idea to create a public fork of Homie. I would prefer to integrate my changes into Homie if they are acceptable by marvin and the homie community. If not, then perhaps I would consider the option of publishing it.

In any case the change required for allowing the nodes to receive service even when no LAN connection is very simple. In file src/Homie/Boot/BootNormal.cpp from line 777 it looks like:

      itoa(_uptime.getSeconds(), uptimeStr, 10);
      Interface::get().getLogger() << F("  • Uptime: ") << uptimeStr << F("s") << endl;
      Interface::get().getMqttClient().publish(_prefixMqttTopic(PSTR("/$stats/uptime")), 1, true, uptimeStr);
      _statsTimer.tick();
    }

    Interface::get().loopFunction();

    for (HomieNode* iNode : HomieNode::nodes) {
      iNode->loop();
    }
  }
}

you have to move the last lines so that the code become

      itoa(_uptime.getSeconds(), uptimeStr, 10);
      Interface::get().getLogger() << F("  • Uptime: ") << uptimeStr << F("s") << endl;
      Interface::get().getMqttClient().publish(_prefixMqttTopic(PSTR("/$stats/uptime")), 1, true, uptimeStr);
      _statsTimer.tick();
    }
  }

  Interface::get().loopFunction();

  for (HomieNode* iNode : HomieNode::nodes) {
    iNode->loop();
  }
}
ortegafernando commented 6 years ago

Hi, what about this? It will be implement? Regards,

tripflex commented 6 years ago

@timpur what are your thoughts about this?

I know my specific project would require something like this as it will still need to function like normal if internet is lost, I just haven't gotten to that point in my project yet, but this will be huge for me

As mentioned in #454 my use case scenario is running a pump, and if internet is lost and pump does not run, plants do not get watered, which could be very bad if spotty internet connection for device

...or septic tank overflows and poop goes everywhere!! haha ... just kidding about that one, I wouldn't use this for something mission critical like that, but you get what i mean 😜

jamesgol commented 6 years ago

I'm still hoping to find the time to throw something together and submit a PR for this. I've actually run into this issue. I had a power outage and my server was in a state that required manual intervention but my temperature control unit (running Homie) didn't function for hours till I got home and fixed the server.

timpur commented 6 years ago

Isn't there a standalone mode already ?

But yes I also think homie needs to work better in the offline area....

jamesgol commented 6 years ago

There is standalone mode but it's not quite the same thing. It's been a while since I dug into it but I believe standalone mode was no network/config mode and it would operate the loop but in regular mode if the network is unavailable it sits and waits.

timpur commented 6 years ago

Think your right. Yeah let's work on this, just need to identify the issues and then some solutions. Let's do this right and no work arounds :P

rakeshpai commented 6 years ago

Thought I'd +1 this.

When I originally created the issue that ended up in the creation of the standalone mode (#125), this is the use-case I had in mind. In the offline case, I was thinking that homie should continue to run as usual, with the understandable tradeoff that all network-related stuff would fail.

The device would have sufficient local data to operate fully. Take the case of a Sonoff running Homie, acting as a timer based light switch. It could have sensible defaults to operate completely locally, without even any network connection. If it connects to the network, it could read timer settings from MQTT to override its defaults, thus allowing an administrator to configure the device over the network. Then, if the network drops for any reason, it could continue to work, since it already has values from MQTT. If for some reason those values get wiped out (say over a reset), it would fall back to using its hardcoded defaults.

This way, the device works even when the network is not available, while using settings it got from MQTT in the past. If it never connected to the network in the first place, it would still have sensible defaults. It wouldn't just lock up, waiting for a network connection.

As it stands, standalone mode doesn't help do this for me. It's a little ironic, since it was my issue that ended up creating the standalone mode in the first place.

timpur commented 6 years ago

I think this will be something worked on in Homie 3.0 since there will be a huge redesign ....

jamesmyatt commented 6 years ago

I don't see the problem here. I put code I want to run all of the time in the normal Arduino loop function and code that I only want to execute when MQTT is connected in the loopHandler (i.e. code to publish data). Isn't that how it's supposed to work?

I wouldn't want the loopHandler to run when not connected to the broker because there's nowhere for the messages to go!

timpur commented 6 years ago

That's what I do also, but I'm only leaving this open to maybe improve this and test that the reconnect sequence doesn't intrupt the main loop or take up valuable CPU time. I guess there are some small things that could be improved to better support this model but yeah still not sure about a new boot mode.

jamesmyatt commented 6 years ago

@timpur , maybe it just needs to be explicit that the loopHandler is for code that only runs while the device is connected to a broker. Might be worth changing some of the examples too.

I guess that you need to check that the loop method doesn't block for too long while reconnecting, otherwise it would stop the proper function of things running outside.

jamesmyatt commented 6 years ago

Given @rakeshpai 's comments (https://github.com/marvinroger/homie-esp8266/issues/354#issuecomment-363309251), maybe there's a case to remove the standalone mode entirely

euphi commented 6 years ago

For library development it would be very useful, if loop() can also be called if Homie is disconnected. That would simplify the usage of HomieNodes as part of a library.

E.g. a temperature Sensor that has impact on other functions (e.g. display or a local relay) would be handled automatically.

I propose to add a bool loopDisconnected to HomieNode that defaults to false. If it is set to true, Homie calls the loop function also if it is disconnected.

I could write a PR for this, but I'm unsure if it should be added to 2.2. It would be possible, because it defaults to a behaviour that is same as current behaviour. WDYT?

For V3 - an integrated scheduler should also add this feature. With the scheduler, I propose to set the "time slice" of the scheduler for "connected" and "disconnected" state seperately.

In my opionion, there is no need to distinguish state further (e.g. "Wifi disconnected" and "MQTT disconnected)

benzino77 commented 6 years ago

@Nzbuu But there is an information in docs that setting loopHandler will call custom loop function in normal mode Have a look here: setLoopFunction and here: Homie modes

jamesmyatt commented 6 years ago

@euphi , Are you proposing to have a library of with HomieNode classes that (for example) sample from the sensor and then publish to MQTT, where you want the sampling to occur even if it's disconnected from MQTT?

If so, then I think that's violating SRP, and I would aim to implement it as two classes: one that samples from the sensor and one that is the HomieNode that just handles the MQTT bit. Then you can loop the first class in the main loop and the HomieNode bit only loops when connected to MQTT.

cecilios commented 6 years ago

@Nzbuu @timpur

I put code I want to run all of the time in the normal Arduino loop function and code that I only want to execute when MQTT is connected in the loopHandler

Probably I have not understood how Homie nodes work. My understanding is that if my code implements a Homie node it is non-sense to split its functionality. The main loop should be just invoking Homie.loop() and Homie should be responsible for doing the support tasks and for invoking the different nodes loop method. The current problem is that Homie does not invoke the nodes loop method when there is not LAN connection. This implies the too strong assumption that nodes can not work properly without LAN connection.

cecilios commented 6 years ago

@Nzbuu Take for example a garden irrigation system. The node must open and close water valves when required, independently of MQTT connectivity. LAN connection is just for changing water times, irrigation duration, etc.

Of course functionality can be splitted: a POD object for holding irrigation data, an Irrigation class for managing irrigation acoording to the data in the POD object, and a HomieNode class for updating the POD object via MQTT. Is really necessary to force Homie users to do this? Wouldn't it be simple to give service to nodes even when no MQTT connectivity?

jamesmyatt commented 6 years ago

@cecilios , Consider what would happen if you wanted to switch from Homie to some other framework, or just remove the MQTT part of your application. If the part of your code that reads from the sensor is coupled to the code that publishes the data via Homie, then you would have to start from scratch. Certainly you could do it all together, but that would be bad software design.

The key parts are:

If you force the loopHandler to run even when MQTT is not connected, then you have to add logic in your loop handler to work out which situation you're in. That's really messy.

jamesmyatt commented 6 years ago

The key thing for me is that Homie only provides (or should provide) an MQTT-based "user" interface for your device, and you need to consider all of the other "user" interfaces, e.g. physical and connected hardware separately.

jamesmyatt commented 6 years ago

@benzino77, "This documentation is valid for Homie v1.5.0"

The development docs are: http://marvinroger.github.io/homie-esp8266/docs/develop/

benzino77 commented 6 years ago

@Nzbuu well, maybe but it is fundamental from the beginning and I have not seen any statement which change this. That's a good assumption that it is still valid. What I usually do is to find docs (those links I provided are from master branch) and then try to follow comments for commits to find out what has been changed in current branch ;)

jamesgol commented 6 years ago

I still think the best way to do this (and it should be easy to implement) is to add a config flag that specifies if the network/mqtt is required to run the loop. This way the default behavior will persist unless someone specifies otherwise.

benzino77 commented 6 years ago

@jamesgol I'm not sure what will be the behavior when you will try to send value for node property when there will be no mqtt connection.

ingoogni commented 6 years ago

Non of my tools (so far) depend on the homie loop, they are fully functional without it. If I add the homie loop I expect it to run and try establish connection etc. independent of what the rest of the tool does. I do not expect a flag or something as adding the homie loop to the main loop is explicit. During development it is just a line to comment out. Tying to send messages without connection does not influence the rest in the main loop.

cecilios commented 6 years ago

@Nzbuu

If you force the loopHandler to run even when MQTT is not connected, then you have to add logic in your loop handler to work out which situation you're in.

When the node needs to transmit data it always must check that LAN is available. Otherwise, as MTTQ uses retained messages, the device could consume all memory with the queue of pending messages. Therefore, that logic is always mandatory. No difference if I split functionality.

Your approach suggests that Homie.setLoopFunction(loopHandler); is useless and the node loopHandler method should always be invoked directly in the main loop. Is then there any gain in using the setLoopFunction?

I always try to apply the single responsibility principle, but only for designing the objetcs/classes. The Node, as an aggregate of classes, will always have the composite behaviour and the SRP has nothing to do in this.

jamesmyatt commented 6 years ago

@cecilios , I'm sorry I didn't explain it better. What I meant was:

If you put all of the functionality into loopHandler, then some of it requires MQTT and some doesn't.

My point was that, if you want some code to run all the time, independently of the MQTT connection, them put it in the arduino loop function, not the Homie loopHandler function. This (IMHO) is the correct way to achieve the functionality that you requested.

The Node objects are the interface layer between the MQTT messages and the physical hardware, and the loop method is use to process the MQTT messages passing through that interface. If the loop method also reads data from a sensor and/or updates the software state representing the system, then that would violate SRP. That is, you are abusing the loop method of the nodes if you attach functionality that must be run even if MQTT is not connected.

cecilios commented 6 years ago

@Nzbuu , Thanks James for your explanations. What I'm trying to say is that, sometimes, it is more difficult/costly to split the functionality, specially taking into account that we are dealing with embedded systems, not too fast, without a multi-tasking operating system, and with very few memory. In this cases it is wise to be pragmatical and not too academic, and to try to save memory, code complexity and running time for a task.

An example of the problem: assume that I follow your suggestion and split the functionality of my code. The node main loop, while doing its work (not requiring MQTT), detects the need to send an MQTT message (e.g to inform about a detected problem or an error). Instead of doing it at that moment (checking previously that there is LAN connectivity) your approach forces to check that there is LAN connectivity and if so, create a 'requests object' to send the MQTT message; this 'request object' will be a memory struct, shared between tasks, with all the data for the message. And continue its job. When the main node loop finish a loop pass, if there is LAN connectivity, the Homie loopHandler method associated to the node will receive service. There, it must check irf the 'request object' is not empty and in taht case, compose the MQTT message and pass it to Homie for sending it. The situation ca be wors if instead of just one messages the number would be random number depending on the conditions. Drawbacks:

And what's the gain for all the associated costs? Nothing, unless you are using TDD design and unit tests (difficult to do with ESP8266). What I'm asking for is something very simple to implement in Homie (a control flag and moving three lines of code out of a loop), without implications for other users not wishing to use this facility.

Anyway, I asked for this seven months ago, and as I could not wait, I already solved my problem. But the suggested change could solve problems to other people using Homie.

jamesgol commented 6 years ago

I'd rather not have to split my loop logic up between code that requires MQTT and code that does not. If I have to then I will, but it makes it more complex than it has to be, requires more clock cycles, and likely a tiny bit more memory.

Right now if you try to call setProperty("").send("") when the network or MQTT is unavailable the code punts right away and generates an error message. Something like: '* SetNodeProperty() impossible now'

I don't think this requires any major rework other than moving the call to the loop outside (as @cecilios said), it just works.

Edit: If this simple change is implemented, then the IteadSonoffButton.ino example is completely functional regardless of network/MQTT availability. Currently with that code if the network is down the button is useless for changing the relay state.

timpur commented 6 years ago

Im split.

If we do code node loops to be 1 to 1 with the arduino loop and only becomes useful as a encapsulation principle, which is beneficial to code design. That said if we do this you can just add a check if connected and continue on as is currently is..... or we can pass the connected state in the loop for convenience. Thinking about this now i think this probs the best option as it allows for both uses, connected or not connected ... and helps encapsulate code.

That said a node should also only contain code related to exposing a property to mqtt. if you have hardware like a screen, this should be independent of the node. ie node and screen should get there data from a independant lib or function. Thus the node loop should only be there to get data from this independant lib.

Need to think about this more, but also thinking maybe it should be up to the person on how the link up all this parts and not force users down uber strict architecture patterns....

euphi commented 6 years ago

Without a scheduler, calling the loop() function of each HomieNode is just a convenience function.

However, it is very useful: Look at the example of my HomieNodeCollection - you just reference the HomieNodes of my lib and everything works out of the box. When you think of a scheduler that is integrated into homie, it will even be more useful to let Homie call a (scheduled) loop function.

I think homie-esp8266 is more than a library for MQTT - it is a framework for automation on ESP8266 controllers. So, please keep in mind that many people using arduino have no awareness for sophisticated SW architecture. They just follow examples and try them out.

So, encapsulation is much more important that SRP.

timpur commented 6 years ago

I lean towards that answers also

jamesmyatt commented 6 years ago

I understand the benefit of being batteries included and of supporting users who don't understand more sophisticated computer science issues, but if homie-esp8266 is trying to do everything, then it's going to get bloated, highly-coupled, confusing and less useful for serious applications, and each component is going to lag behind other libraries that do one thing well.

The scheduler is a good example. It currently just meets the needs of the Homie library itself. If there's an expectation that you can run all of your "loop" code via that, then there's an awful lot to be desired and there are probably existing libraries that already do this well.

I'm pretty sure it's possible to design something that's as easy to use as @euphi 's HomieNodeCollection (which does look good BTW), but has better separation of concerns. If it's going to be a generic high-level framework then it should encourage good patterns, while still supporting simple cases easily. I absolutely agree about the importance of the examples. So if the examples don't match best practice, then they need updating urgently.

On the specific point above about not wanting to splitting logic, if you want to run all of it regardless of MQTT connection, then you can just put it in the main loop and not have a loopHandler at all. If the issue is implementing nodes by derived HomieNode, then this is already an advanced topic and we should not be discarding good design for the sake of one particular use case.

I guess the TL;DR is, if the current Homie loop method is supposed to be a generic scheduler, then that needs to be upgraded, rather than hacked.

timpur commented 6 years ago

God, idk what's the right answer. I do wonder if @Nzbuu is right and if you need this one to one with Arduino look then maybe just add your own mainLoop function to the class and call that from aruino loop ( as your own hack). But I do think trying to follow best design patterns is best. If we can do this in a way that is also easy for new people to pick up then that's amazing. Unknowingly forcing people to use good design patterns is a good thing :P.

I think that this issue might be addressed better in 3.0 where a task seceduler will be used in homie(it already uses one, just exsposing it and streamlining) but also making it easy for users to use.

I'll also consalt the Oracle and see if he has any idea on this.

There is lots of concern that homie is trying to do to much and getting bloated. Hence 3.0 is trying to introduce new features as add-ons from the core. Not saying this change will do that but the ideology we have for homie will.

jamesmyatt commented 6 years ago

I'm very much in favour of a more add-on oriented architecture

euphi commented 5 years ago

Closed with #490.