Porting MySensors to work with the RadioHead library
-
@kolaf What version of arduino IDE do you use? you should really use 1.5+
(But I'll fix so it works in older version) -
I have been thinking on how to extend the library to support different radios. As far as I can see there are two options.
-
Have a generic radio driver pointer (of the superclass type RHGenericDriver) and initialise this depending on an input parameter to the Gateway constructor. The trouble with this approach is that we have to static_cast every access to this variable to its appropriate subclass in order to access radio specific functions to set transmit power, et cetera. We also have to include a bunch of header files so that everything is available during runtime. This obviously increases footprint and complexity for the maintainer.
-
Use a define in the sensor application and Gateway application to determine which radio is to be used, e.g. "#define DRH_RF69" to select the radio I am using. With an include #ifdef directives in the header and my sensor source files to include only the necessary driver headers and choose the correct set of initialisation functions for the radio. Most of this would go into the setupRadio function.
I think that the second approach is the easiest to implement. It also requires a single addition to any client application, namely the correct define at the top of the configurations header file. We can leave this as a default radio value, and the knowledgeable user can change this if he uses a different chipset. Does this make sense?
-
-
I have been thinking on how to extend the library to support different radios. As far as I can see there are two options.
-
Have a generic radio driver pointer (of the superclass type RHGenericDriver) and initialise this depending on an input parameter to the Gateway constructor. The trouble with this approach is that we have to static_cast every access to this variable to its appropriate subclass in order to access radio specific functions to set transmit power, et cetera. We also have to include a bunch of header files so that everything is available during runtime. This obviously increases footprint and complexity for the maintainer.
-
Use a define in the sensor application and Gateway application to determine which radio is to be used, e.g. "#define DRH_RF69" to select the radio I am using. With an include #ifdef directives in the header and my sensor source files to include only the necessary driver headers and choose the correct set of initialisation functions for the radio. Most of this would go into the setupRadio function.
I think that the second approach is the easiest to implement. It also requires a single addition to any client application, namely the correct define at the top of the configurations header file. We can leave this as a default radio value, and the knowledgeable user can change this if he uses a different chipset. Does this make sense?
-
-
@kolaf said:
I think that the second approach is the easiest to implement
Agree. Pointers to instances makes more sense when radios have to be switched dynamically or when multiple radios have to be supported.
For this application having a #define in a configuration file makes perfect sense. -
I am also happy to report that I just tried to compile the secret knock sensor without any difficulties :-)
Sketch uses 22,074 bytes (68%) of program storage space. Maximum is 32,256 bytes. Global variables use 1,241 bytes (60%) of dynamic memory, leaving 807 bytes for local variables. Maximum is 2,048 bytes.@kolaf said:
Sketch uses 22,074 bytes (68%) of program storage space. Maximum is 32,256 bytes.
Global variables use 1,241 bytes (60%) of dynamic memory, leaving 807 bytes for local variables. Maximum is 2,048 bytes.These are very acceptable numbers IMHO.
No need to worry about flash/ram usage! -
@kolaf said:
I think that the second approach is the easiest to implement
Agree. Pointers to instances makes more sense when radios have to be switched dynamically or when multiple radios have to be supported.
For this application having a #define in a configuration file makes perfect sense.@Yveaux said:
Pointers to instances makes more sense when radios have to be switched dynamically or when multiple radios have to be supported.
Yes, defines certainly make sense now! But it would be cool to have gateway-nodes between different radio models in the future ;)
-
@Yveaux said:
Pointers to instances makes more sense when radios have to be switched dynamically or when multiple radios have to be supported.
Yes, defines certainly make sense now! But it would be cool to have gateway-nodes between different radio models in the future ;)
@hek Maybe, but I have yet to see a valid usecase for this. You can always use 2 Arduino's running the default serial gateway and tie them together ;-)
I haven't looked into the radiohead interface yet (have to admit you guys got me curious, though) but isn't there simply one base class defining the interface and every radio derives from it? That would definately make things simple!http://yveaux.blogspot.nl
-
@hek Maybe, but I have yet to see a valid usecase for this. You can always use 2 Arduino's running the default serial gateway and tie them together ;-)
I haven't looked into the radiohead interface yet (have to admit you guys got me curious, though) but isn't there simply one base class defining the interface and every radio derives from it? That would definately make things simple! -
@Yveaux said:
Maybe, but I have yet to see a valid usecase for this
Yeah, the use case is a bit strained and theoretical. But you know how it is... eventually someone want to try to mix different brand radio units. ;)
-
I'm getting a bunch of errors when I compile the mqtt gateway source related to the initial list of constants. The error messages look like this:
D:\Home control\Arduino\libraries\MySensors\MyMQTT.cpp:14:15: error: variable 'broker' must be const in order to be put into read-only section by means of '__attribute__((progmem))' char broker[] PROGMEM = MQTT_BROKER_PREFIX; ^ D:\Home control\Arduino\libraries\MySensors\MyMQTT.cpp:16:12: error: variable 'S_0' must be const in order to be put into read-only section by means of '__attribute__((progmem))' char S_0[] PROGMEM = "Temperature"; //V_TEMP ^ D:\Home control\Arduino\libraries\MySensors\MyMQTT.cpp:17:12: error: variable 'S_1' must be const in order to be put into read-only section by means of '__attribute__((progmem))' char S_1[] PROGMEM = "Humidity"; //V_HUMMy current solution is to just delete these two files since I only care about the serial gateway, but it would be good to have everything working :-).
-
@hek Maybe, but I have yet to see a valid usecase for this. You can always use 2 Arduino's running the default serial gateway and tie them together ;-)
I haven't looked into the radiohead interface yet (have to admit you guys got me curious, though) but isn't there simply one base class defining the interface and every radio derives from it? That would definately make things simple!@Yveaux said:
I haven't looked into the radiohead interface yet (have to admit you guys got me curious, though) but isn't there simply one base class defining the interface and every radio derives from it? That would definately make things simple!
Unfortunately it is not as simple as that. Each radio driver has a constructor which is completely different from the others, and they have different concepts.
For instance, the RF 69 library requires an interrupt pin and a select pin, while the NRF 24 library uses a select pin and an enable pin. They also have completely different notions of frequency versus channel, modem modulation versus data rate, and so forth.
I think we should expand the setupRadio function to configure some useful defaults for every radio, and then we can expose the specific radio driver so that the power user can set more complex options such as a different modulation, data rate, channel, et cetera.
Make sense?
-
@kolaf said:
I think we should expand the setupRadio function to configure some useful defaults for every radio
Can't you wrap the radio setup and some more stuff in a class for every radio type, derived from a single interface class?
I hope in the actual mesh layer etc. you don't have to distinguish between different radio types anymore.... -
@kolaf said:
I think we should expand the setupRadio function to configure some useful defaults for every radio
Can't you wrap the radio setup and some more stuff in a class for every radio type, derived from a single interface class?
I hope in the actual mesh layer etc. you don't have to distinguish between different radio types anymore....@Yveaux said:
@kolaf said:
I think we should expand the setupRadio function to configure some useful defaults for every radio
Can't you wrap the radio setup and some more stuff in a class for every radio type, derived from a single interface class?
I hope in the actual mesh layer etc. you don't have to distinguish between different radio types anymore....I guess I could do that, but it sounds like more work :-). For now I will stick to a define section for each radio that configures the defaults. After all, most of the radio settings for the original version of the library also relies on defines.
I think the RF 69 configuration of the library is ready for testing by others, and I have also verified that the NRF 24 version of the library compiles. I do not have any hardware for this one, so maybe someone else wants to test to see if it works? I have forked the original repository, and my current progress can be found in the following branch (Radiohead_port):
-
If you want to test, be aware that the default pins for the constructor set up to match the RF 69 radio. You have to specify the select and enable pins in the constructor explicitly.
-
I just had a quick look at the RadioHead implementation.
All drivers are derived from RHGenericDriver (from http://www.airspayce.com/mikem/arduino/RadioHead/classRHGenericDriver.html):
If you pass an instance of RHGenericDriver in e.g. the c'tor of MySensors you're done.... But maybe I had a too-quick look ;-)
-
It is not that simple because all the different specific drivers implement a bunch of their own functions which are not virtualised. This means that you need to static cast or dynamic cast the driver every time you want to use it which is a real pain.
On the other hand, each driver seems to come with a relatively intelligent set of defaults, so I think it is quite easy to cater for the generic case, and then we can expose the internals for those who really want to get into the specifics. Still, if you want to try to generalise it in a different way, feel free :-)
-
It is not that simple because all the different specific drivers implement a bunch of their own functions which are not virtualised. This means that you need to static cast or dynamic cast the driver every time you want to use it which is a real pain.
On the other hand, each driver seems to come with a relatively intelligent set of defaults, so I think it is quite easy to cater for the generic case, and then we can expose the internals for those who really want to get into the specifics. Still, if you want to try to generalise it in a different way, feel free :-)
@kolaf said:
It is not that simple because all the different specific drivers implement a bunch of their own functions which are not virtualised. This means that you need to static cast or dynamic cast the driver every time you want to use it which is a real pain.
Are those driver-specific functions needed during normal operation, or are they only for non-default setup? (eg: frequency, bandwidth, etc)
-
What about packet size? I'm guessing that the idea is to use the 32 byte limit of the nRF24L01+ for all radios (even if they have larger packet options) right?
(This approach is frequently called "least common denominator", tho the actual mathematical concept being misreferenced is actually the greatest common denominator)
-
I don't think the flash and RAM footprint is a deal breaker in this application, but it could be a concern for some multi-sensor nodes which include several sensor libraries. So I was wondering how much heavier it is, but not saying (yet) that it's too large.
Remember to save room for a possible crypto library, if we do a more secure version.
My own use case is unusual. I am using nRF24L01+ for christmas light control, which involves a high bandwidth transmission from hub to nodes (and uses a fair amount of RAM on the nodes for buffers). I've been thinking that I could have the Christmas light display nodes also transmit sensor data to a MySensors network periodically,since they use the same radio and processor. That isn't critical tho. I could just use separate nodes on different channels for the two applications.
-
What about packet size? I'm guessing that the idea is to use the 32 byte limit of the nRF24L01+ for all radios (even if they have larger packet options) right?
(This approach is frequently called "least common denominator", tho the actual mathematical concept being misreferenced is actually the greatest common denominator)
@Zeph said:
I'm guessing that the idea is to use the 32 byte limit of the nRF24L01+ for all radios (even if they have larger packet options)
Why? MySensors can easily handle payloads with different sizes. Only when you start mixing different radios in one network ( hek apparently thinks in this direction) you have to handle fragmentation.
