Porting MySensors to work with the RadioHead library

Yveaux

@kolaf I totally agree with you.
The MySensors library currently has a solid nrf24 implementation and routing works fine (though not fully mesh) so there's no direct need to switch driver and routing layer.
This is a roadmap item on which we should continue working, IMO together with the RadioHead development. The RadioHead library has only recently been developed (few months old or so) and is already very mature looking. This is very promising for the future and there's no use in developing/maintaining 2 nearly identical libraries.
I think the current flaws (e.g. code & message size) can be improved with our help making it a solid base for MySensors.

ToSa

Thanks @kolaf @Yveaux.

makes sense - I thought "just" using the drivers at least as an interim step could give us a head-start but it appears that the majority of the "issues" with RH need to be fixed on the driver level anyways so we would still need to work these first with the RH team...

@Yveaux said:

Ok, read a bit through the docs. Apparently this is what is sent:
RHDatagram:  ff:7e:01:00     (TO:FROM:ID:FLAGS)
RHRouter:    ff:7e:00:52:00  (DEST:SOURCE:HOPS:ID:FLAGS)
RHMesh:      01:01:00        (ROUTE_DISCOVERY_REQUEST:<RESERVED>:DEST)
Therefore the destination and source address are sent TWICE (destination even 3 times with nRF24, as the destination address is part of the nRF24 header). An ID byte (incremented with each message sent) is also present in RHDatagram & RHRouter.
After the route is known, sending through RHMesh still requires 1 byte to indicate application payload is transmitted.
For short, Every message will require 10 bytes header, at least, to which the MySensors payload is added (including its own header of currently 4 bytes) giving at least 14 bytes overhead of 32 bytes total.
To me this feels like too much....

I think the addressing is not that bad: the TO/FROM is actually the next/last used in MySensors - only for a single-hop communication these are the same as SOURCE/DEST. With that FROM/SOURCE/DEST in RH == last/sender/destination in MySensors. The TO (next) is the one that is unnecessary because it's part of the nRF24 header - as this is not the case for all radios, this should be adjusted in the NRF24 specific code in RH and shouldn't be that hard to do.

The ID fields are used to determine if a packet is a duplicate (once for the hop and once end-to-end). I'm wondering how this is done in MySensors today. For the single hop that's probably part of the nRF24 internals (auto-acknowledge / auto-resend) and maybe it could be avoided on the driver level in RH but probably needs some tweaking of ReliableDatagram as well. For e2e I don't think there is an equivalent in MySensors today and it might actually be a valuable add rather than a waste of a byte :) what could happen without it: a node receives a packet and sends an ack - the ack does not make it to the sender and therefore the sender submits the same packet again - the node receives the package once again and thinks it's a new packet... think about a command that toggles a light - woulc toggle twice and go off->on->off instead of off->on.

The FLAGS bytes appear to be a total waste of space.

Looking at the first FLAGS byte it's actually defined on the driver level already but not used in Driver or Datagram but only in ReliableDatagram and the only flag set is RH_FLAGS_ACK - a full byte for a single bit worth of information... The two options would be to either remove that byte and mark ACK packets differently or to just make use of the remaining 7 bits for other purposes. I would prefer using the 7 bits - protocol version (3 bits) is definitely a good fit - command (3 bits) might be the other. Using the lower four bits of FLAGS for application layer purposes is even foreseen in RH (and could be easily changed to use 6 bits for app layer in RHGenericDriver.h)
The benefit of the second FLAGS byte (from the Router) is totally unclear to me. It seems like the neither RHMesh nor RHRouter code uses it at all even if it's defined on that level so nothing else should use it either... worth a discussion with the RH team - and either remove it completely or use it again for app layer specific stuff (MySensors type?). I'm surprised that they don't use these FLAGS instead of the first byte of RHMesh messages to determine route discovery vs. app date vs. ...

Assuming that we can get rid of the "TO" and the "driver level ID" and make use of the FLAGS fields as mentioned above the header would be:

RHDatagram: FROM:FLAGS (covering MyMessage last / ack / version / command)
RHRouter: DEST:SOURCE:HOPS:ID:FLAGS (covering MyMessage destination / sender / type)
RHMesh: one byte to determine "application data" - tbc if this can be moved to the FLAGS instead consuming less than a full byte. at least the MyMessage "payload type" can be included in that byte
MyMessage header: none (would need to talk to @hek but I think MyMessage::sensor should actually not be part of the header anymore but part of specific message types only).

This would leave us with 2+5+1=8 bytes instead of the current 7 bytes for the header (to be fair 8 instead of 6 due to the removed sensor field). That's not as bad as 14!

hek

@ToSa said:

MyMessage header: none (would need to talk to @hek but I think MyMessage::sensor should actually not be part of the header anymore but part of specific message types only).

Yes, that makes sense.

hek

Reply from Mike:

Some of your contributors have the wrong idea about the headers.

There are 4 header bytes used by drivers, RHDatagram and RHReliableDatagram

TO
FROM
ID
FLAGS

These are the hop-to-hop headers. In fact they are present in the payload (but
effectively unused) even if you use the drivers directly.

RHRouter and RHMesh (if you use them) add

DEST
SOURCE
HOPS
ID
FLAGS

these are end-to-end headers and are not necessarily the same as the hop-to-
hop headers. In the general case they will be different values.

In my view all these header are all necessary.

BTW, new version 1.33 supports sleep mode for RH_RF69, RH_RF22, RH_NRF24,
RH_RF24, RH_RF95 drivers.

Cheers.

ToSa

@hek said:

Reply from Mike:

these are end-to-end headers and are not necessarily the same as the hop-to-
hop headers. In the general case they will be different values.

yep :)

In my view all these header are all necessary.

Well, debatable at least for the FLAGS (or the size of the FLAGS).

Three questions (@Mike, if you are reading here, just reply directly, otherwise I'll contact you via the RH channels...):

is the RH library itself using the end-to-end FLAGS? I couldn't find any reference other than setting it to 0 (when using RHMesh).
is the RH library itself using the hop FLAGS for anything other than identifying ACK packets (RH_FLAGS_ACK / RH_FLAGS_NONE)?
for the hop ID and TO - if the specific RF chip supports this internally (as part of the preamble/header the chip adds to the on-air packet), why duplicating this wasting valuable payload size - the driver for the specific RF chip could handle it internally and make it look the same for the outside world using the RHGenericDriver interface without changes... I'll give it a try and let you know.

Yveaux

@hek Thanks for getting Mike 'on board'! Good move!

kolaf

Great work guys. I'm glad to see some enthusiasm for this as it will allow me to utilise the platform and not have to reinvent the wheel :-)

ToSa

Test compilation comparing code size with current MySensors 1.4 radio library and with radiohead. The radiohead integration to MySensors is not optimized (pushing full MySensors header through even if the radiohead header has the same content etc.) and the radiohead library provides some benefits (not just a 1:1 replacament) - so it's expected to be larger. Based on the two examples below it's right now about 4-5k flash and ~800 ram for global variables.

=> let's see how far we can get that down reducing the duplication of header data etc.

**SerialGateway with MySensors: **
Sketch uses 17,566 bytes (57%) of program storage space. Maximum is 30,720 bytes.
Global variables use 688 bytes (33%) of dynamic memory, leaving 1,360 bytes for local variables. Maximum is 2,048 bytes.

SerialGateway with RadioHead:
Sketch uses 22,242 bytes (72%) of program storage space. Maximum is 30,720 bytes.
Global variables use 1,455 bytes (71%) of dynamic memory, leaving 593 bytes for local variables. Maximum is 2,048 bytes.

**DallasTemp with MySensors: **
Sketch uses 20,288 bytes (66%) of program storage space. Maximum is 30,720 bytes.
Global variables use 551 bytes (26%) of dynamic memory, leaving 1,497 bytes for local variables. Maximum is 2,048 bytes.

DallasTemp with RadioHead:
Sketch uses 24,592 bytes (80%) of program storage space. Maximum is 30,720 bytes.
Global variables use 1,319 bytes (64%) of dynamic memory, leaving 729 bytes for local variables. Maximum is 2,048 bytes.

Damme

@ToSa Wonder what the runtime values are...

Yveaux

@ToSa said:

SerialGateway with RadioHead:
Sketch uses 22,242 bytes (72%) of program storage space. Maximum is 30,720 bytes.
Global variables use 1,455 bytes (71%) of dynamic memory, leaving 593 bytes for local variables. Maximum is 2,048 bytes.

What radio/Arduino IDE did you compile for?

For Serial Gateway, nrf24, Arduino 1.5.7 I get:
Sketch uses 20.962 bytes (64%) of program storage space. Maximum is 32.256 bytes.
Global variables use 1.471 bytes (71%) of dynamic memory, leaving 577 bytes for local variables. Maximum is 2.048 bytes.

@Damme

Wonder what the runtime values are...

Currently MySensor::setRadio creates an instance of RHMesh at runtime, so RAM usage is even worse...

Damme

This post is deleted!

ToSa

@Yveaux said:

What radio/Arduino IDE did you compile for?

nRF24 / 1.5.6-r2
DEBUG turned on - your values appear to be with DEBUG turned off

I did a few changes starting to reduce the header so I can't rerun with DEBUG off and compare right now...

kolaf

A small digression from the main topic here, but I just tested the mesh functionality of the library. I took a laptop with a serial gateway outside of the range of my sensor and observed that it stopped working. I then went back a few metres and powered up a battery powered sensor which basically just sent hello every second and processed messages. I was able to significantly extend my range, experiencing only a few seconds of interrupt in the ping flow.

Moving back and powering down the intermediate node, the direct route was reestablished within a new couple of seconds and everything was working correctly again :-) I think I will have much fun playing with this.

Yveaux

@kolaf said:

I was unable to significantly extend my range, experiencing only a few seconds of interrupt in the ping flow.

Shouldn't that be "I was able"?
I want to be happy with you, but not sure now ;-)

kolaf

You are most certainly correct, I have edited my original post. I'm suffering a bit from tendinitis, so I'm using speech recognition when I write. Sometimes it recognises almost exactly what I say ;)

Zeph

@Yveaux said:

The MySensors library currently has a solid nrf24 implementation and routing works fine (though not fully mesh) so there's no direct need to switch driver and routing layer.

What are the differences between the MySensors protocol and RH in terms of mesh dynamics?

I'm guessing that RH is more dynamicly configured, but I'd like to understand more.

Yveaux

@kolaf ah, great to see your results then!
I read some mixed results on the Radiohead mailing list regarding mesh, so good to see it's working for you.
BTW I'm in the progress of writing wireshark directors for Radiohead which can be used with the nrf24 sniffer. It'll give us better insight on Radiohead's performance and functioning.

Zeph

It makes sense to migrate the MySensors last,sender and destination bytes to RH from,source,dest.

I do not think it makes sense to move type (V_code) or command into the RH FLAGs. Typically when such space is reserved for the protocol in the header but not yet fully used, it's subject to change as the protocal evolves. If part of the goal is to take advantage of the maintenance and ongoing development of RH, we don't want to set up a conflict - because other users are not going to use those FLAGs our way anyway.

Basically, Radio Head should be concerned with delivering a payload (set of bytes) to a node. Everything specific to just the MySensors application should be in the payload (as seen by RH). That includes:

child id ("sensor")
command
V_code ("type")
version (if needed)
ack - is this still needed??

If we want to remain compatible with other RH users (ie: their development), rather than re-allocating bytes or bits in the RH header to our own purposes, we would be putting our MySensors' header in the RH payload, but omitting from our header the redundant fields which are handled by RH:

last,
sender,
destination

The RH payload would include the reduced MySensors header plus the MySensors payload.

ToSa

@Zeph said:

I do not think it makes sense to move type (V_code) or command into the RH FLAGs. Typically when such space is reserved for the protocol in the header but not yet fully used, it's subject to change as the protocal evolves. If part of the goal is to take advantage of the maintenance and ongoing development of RH, we don't want to set up a conflict - because other users are not going to use those FLAGs our way anyway.

From the RH documentation / the source code:

for Datagram FLAGS:
A bitmask of flags. The most significant 4 bits are reserved for use by RadioHead. The least significant 4 bits are reserved for applications.
for Router/Mesh FLAGS:
Optional flags for use by subclasses or application layer

Not setting up a conflict with the RH code will be the biggest issue: the two benefits of RH are the multi radio support and the more advanced Mesh topology - which would be worth a couple of additional bytes in flash and ram - but the other reason for the big overhead is that several advanced features of the nRF24 chip are not used because they are not available in all supported chips.

Two examples:

the multi-pipe capability of nRF24 would allow to filter traffic to only current address and broadcast and avoid that any other traffic ever reaches the MCU (only slightly less code but wondering if that helps with battery powered nodes).
the ReliableDiagram processing in code essentially duplicates capabilities that are build into the chipset as well (auto-acknowledge / autoresubmit)

I know you are interested in getting RF69 supported but the question is how much negative impact for all the other users (with current nRF24 setup) is acceptable. That's like using the RF69 and implementing an AES encryption in code rather than using the build-in capabilities of the chip...

ToSa

Didn't want to sound too negative yesterday...

I'm still looking at the RH library but changes will be major and will not necessarily be in line with the RH purpose so Mike might not want to merge them into the main codebase: mainly defining what driver is used at compile time of the library (similar to MyConfig) which would then allow to e.g. "bypass" the majority of ReliableDatagram etc.
what we should definitely do is splitting application layer and network layer: no longer inherit MySensor from RF24 but create an RF24 instance at runtime / leave application layer message handling in MySensors / move network layer message handling to a separate class (the "driver" for RF24). This will make it way easier to switch radios in the future.

Porting MySensors to work with the RadioHead library

7

11.7k

11.2k

113.0k