Porting MySensors to work with the RadioHead library

[hek]

Ok, tried to clean up some of the mess in RFM69 (and gw node id).

I have a hard time to do any verification tonight here (the family sleeping too near the "lab room"). Would be great to get some feedback if you have your gears setup and ready for tests.

[reddy11]

@hek
Thank you for taking a look at the RFM69 code. I downloaded the development branch. While I was able to get it to compile, it does not seem to work, but Im note sure if it is something I am doing.
Background:
I had the RFM69 radio working with code from the development branch I downloaded in early Jan. I have a few sensors that and a serial gateway using the Jan code.

After downloading the latest development branch I tried to update my serial gateway, assuming it would still be able to read the sensors with the older code.

Observations:
Currently I cannot see any of the older nodes that I have. I have not tried to update any of the nodes yet, so I don't know if my problem is with the current code or not.
Currently there is not a myconfig.h option to set the RFM69HW option

I have been stumbing around the code trying to get a feel for it, and I am not and expert but am wondering if Line 137 of Mysensor.h has anything to do with the problem?

	MySensor(MyTransport &radio =*new MyTransportNRF24(), MyHw &hw=*new MyHwDriver()

Should there be something that would define MyTransportRFM69 instead?

Thoughts?

Would love to get this working and ultimately figure out the MQTT server options.

Thanks!

[hek]

Yes, it differs a bit. You can now initialise the transport using RFM69 like this (see constructor default values in https://github.com/mysensors/Arduino/blob/development/libraries/MySensors/MyTransportRFM69.h.
5:th argument is the one you're looking for. I Might add a default for this in MyConfig as well if this is something people change often.

#include <MyTransportRFM69.h>
#include <MySensor.h>

MyTransportRFM69 transport(freqBand, networkId,  slaveSelectPin, interruptPin, isRFM69HW, interruptNum);

MySensors gw(transport);

[kolaf]

@hek I updated with your latest changes and refreshed my node and gateway (which I am happy to report has run stable since yesterday). Things seem to work out of the box now which is good. There is a small issue, I think, where you could use the GATEWAY_ADDRESS a few more places for consistency, but apart from that I'm quite happy :-).

diff --git a/libraries/MySensors/MySensor.cpp b/libraries/MySensors/MySensor.cpp
index 70d3680..4a87e33 100755
--- a/libraries/MySensors/MySensor.cpp
+++ b/libraries/MySensors/MySensor.cpp
@@ -101,9 +101,9 @@ void MySensor::begin(void (*_msgCallback)(const MyMessage &), uint8_t _nodeId, b
 
 	if (isGateway) {
 		// Set configuration for gateway
-		nc.parentNodeId = 0;
+		nc.parentNodeId = GATEWAY_ADDRESS;
 		nc.distance = 0;
-		nc.nodeId = 0;
+		nc.nodeId = GATEWAY_ADDRESS;
 	} else {
 		// Read settings from eeprom
 		hw_readConfigBlock((void*)&nc, (void*)EEPROM_NODE_ID_ADDRESS, sizeof(NodeConfig));

[hek]

@kolaf
Sound good.

[A Former User]

@kolaf and @hek
Thanks for the great work. I will try to setup a test node and gateway by tomorrow and let it run over the Easter weekend. Hopefully it still runs when I am back :)

[kolaf]

@hek I tried setting up a second sensor (temperature and power supply voltage) with the updated library. I wiped eeprom, uploaded the sketch, and fired it up.

It requested a node ID from the gateway, which it received, and everything seemed to go smoothly until it had transmitted the first data point. After that it immediately enters a request for a new node ID, which it receives, and things go in an endless loop. Here is the log:

find parent
send: 255-255-255-255 s=255,c=3,t=7,pt=0,l=0,sg=0,st=bc:
read: 0-0-255 s=255,c=3,t=8,pt=1,l=1,sg=0:0
parent=0, d=1
req id
send: 255-255-0-0 s=255,c=3,t=3,pt=0,l=0,sg=0,st=ok:
read: 0-0-255 s=255,c=3,t=4,pt=0,l=1,sg=0:1
send: 1-1-0-0 s=255,c=0,t=17,pt=0,l=5,sg=0,st=ok:1.4.1
send: 1-1-0-0 s=255,c=3,t=6,pt=1,l=1,sg=0,st=ok:0
id=1
send: 1-1-0-0 s=255,c=0,t=17,pt=0,l=5,sg=0,st=ok:1.4.1
send: 1-1-0-0 s=255,c=3,t=6,pt=1,l=1,sg=0,st=ok:0
sensor started, id=1, parent=0, distance=1
send: 1-1-0-0 s=255,c=3,t=11,pt=0,l=20,sg=0,st=ok:Temperature basement
send: 1-1-0-0 s=255,c=3,t=12,pt=0,l=3,sg=0,st=ok:1.0
send: 1-1-0-0 s=1,c=0,t=6,pt=0,l=5,sg=0,st=ok:1.4.1
send: 1-1-0-0 s=2,c=0,t=13,pt=0,l=5,sg=0,st=ok:1.4.1
1023send: 1-1-0-0 s=2,c=1,t=38,pt=7,l=5,sg=0,st=ok:5.4
send: 1-1-0-0 s=255,c=3,t=0,pt=1,l=1,sg=0,st=ok:102
send: 1-1-0-0 s=1,c=1,t=0,pt=7,l=5,sg=0,st=ok:20.6
req id
send: 1-1-0-0 s=255,c=3,t=3,pt=0,l=0,sg=0,st=ok:
read: 0-0-1 s=255,c=3,t=4,pt=0,l=1,sg=0:3
send: 1-1-0-0 s=255,c=0,t=17,pt=0,l=5,sg=0,st=ok:1.4.1
send: 1-1-0-0 s=255,c=3,t=6,pt=1,l=1,sg=0,st=ok:0
sensor started, id=1, parent=0, distance=1
send: 1-1-0-0 s=255,c=3,t=11,pt=0,l=20,sg=0,st=ok:Temperature basement
send: 1-1-0-0 s=255,c=3,t=12,pt=0,l=3,sg=0,st=ok:1.0
send: 1-1-0-0 s=1,c=0,t=6,pt=0,l=5,sg=0,st=ok:1.4.1
send: 1-1-0-0 s=2,c=0,t=13,pt=0,l=5,sg=0,st=ok:1.4.1
955send: 1-1-0-0 s=2,c=1,t=38,pt=7,l=5,sg=0,st=ok:5.0
send: 1-1-0-0 s=255,c=3,t=0,pt=1,l=1,sg=0,st=ok:95
send: 1-1-0-0 s=1,c=1,t=0,pt=7,l=5,sg=0,st=ok:20.7
req id
send: 1-1-0-0 s=255,c=3,t=3,pt=0,l=0,sg=0,st=ok:
read: 0-0-1 s=255,c=3,t=4,pt=0,l=1,sg=0:3
send: 1-1-0-0 s=255,c=0,t=17,pt=0,l=5,sg=0,st=ok:1.4.1
send: 1-1-0-0 s=255,c=3,t=6,pt=1,l=1,sg=0,st=ok:0
sensor started, id=1, parent=0, distance=1
send: 1-1-0-0 s=255,c=3,t=11,pt=0,l=20,sg=0,st=ok:Temperature basement
send: 1-1-0-0 s=255,c=3,t=12,pt=0,l=3,sg=0,st=ok:1.0
send: 1-1-0-0 s=1,c=0,t=6,pt=0,l=5,sg=0,st=ok:1.4.1
send: 1-1-0-0 s=2,c=0,t=13,pt=0,l=5,sg=0,st=ok:1.4.1
956send: 1-1-0-0 s=2,c=1,t=38,pt=7,l=5,sg=0,st=ok:5.0
send: 1-1-0-0 s=255,c=3,t=0,pt=1,l=1,sg=0,st=ok:95
send: 1-1-0-0 s=1,c=1,t=0,pt=7,l=5,sg=0,st=ok:20.7

As far as I can see this should be caused by the node ID still being AUTO, but this shouldn't be the case since it appears to be correctly stored in ROM?

[kolaf]

Please disregard my previous "analysis"/guess. It turns out that the repeating behaviour that was caused by a program crash. The trigger for this crash is MyHwATMega328::sleep(ms) . I'm running a RFM69HW radio on a anarduino board, which should be using the ATMega 328 chip, same as the moteino.

By replacing the call to this function in MySensor::sleep(unsigned long ms) with a simple delay(ms), the crashes no longer occur.

[kolaf]

My current guess is that the watchdog timer resets the chip after the time period instead of throwing an interrupt to wake the system...?

[kolaf]

I narrowed it down even further. It appears that there is no appropriate interrupt and present to handle when the watchdog fires. I have no idea why this is the case, and I do not know if this fix is correct, but by a placing this code somewhere in MySensors.cpp everything works nicely:

ISR( WDT_vect ) {
    /* dummy */
}

[hek]

@kolaf

Ok, strange. Would be nice to know why this fixes the problem,

It should probably be placed here if it's atmega328 related (and only need to be called once):
https://github.com/mysensors/Arduino/blob/development/libraries/MySensors/MyHwATMega328.h#L22

[kolaf]

@hek
As far as I know the function is the interrupt routine that is called by the watchdog when the time expires. I guess it doesn't matter where it is defined so it should be fine to move it. It is not called explicitly anywhere in the code, but by the watchdog. As such, I don't think it should be defined as is done in the header file for the other functions.

My question is why is it not working for me without this code, I assume it has been working for everyone else?

[hek]

Ah.. doh..

Moved parts of the low-power library into MyHwATMega328 and forgot the ISR-thingie.

https://github.com/mysensors/Arduino/blob/master/libraries/MySensors/utility/LowPower.cpp#L850

Good catch.

[kolaf]

Glad I could help :-). I guess my prize will be that you patch this so that I do not have to create a pull request ;)

[kolaf]

I might as well continue while I'm at it :-)

Another weird issue I'm having is that every node requests a new ID whenever it starts, but it does not switch to the new ID it receives. It just continues with the old ID it already had.

When I wipe the ROM it requests a new ID as expected, and this time it is saved and used for subsequent communication. Why is it requesting a new ID when it boots, when it already has one?

[hek]

@kolaf

Should not happen. Will look at it later tonight.

[hek]

Ok, finally got the thumbs out and actually tested the RFM69 code using a couple of Moteinos. They seem to communicate just fine. :)

Tested with SerialGateway<->RelayActuator-Example.

[reddy11]

I can also report success and things look pretty good. Had to change the line:

//MyTransportRFM69 transport;
MyTransportRFM69 transport(RFM69_FREQUENCY, RFM69_NETWORKID, RF69_SPI_CS, RF69_IRQ_PIN, true, RF69_IRQ_NUM);

So that the rfm69HW could be supported.

Now, on to see if I can get the MQTT working.

[kolaf]

@reddy11 I had to do the same, although I did the change in the default parameters for the function definition since all my radios are HW :-).

I'm not using the MQTT gateway, I'm using the serial gateway together with a Perl script someone here made which functions as a MQTT gateway, and which I modified to handle node ID assignments.

[kolaf]

@hek You didn't see any extra node ID requests when the sensor booted?