Two RFM69 on One Moteino

[kiwisincebirth]

To follow up on my statements I previously made about interrupt routines needing to be static. Please read the following, it explains it better than I can:

http://stackoverflow.com/questions/15669017/cant-use-attachinterrupt-in-a-library

For a partial solution I have attached a RFM69.cpp and RFM69.h file which should (but untested) resolve the interrupt handling issue, between two instances of the Class. The main thing is you need to specifically instantiate the second instance with the main constructor passing in the appropriate pin numbers and setting the interruptNum=1. If you search the code for 'selfPointer1' and 'isr1' you can see where I have made the changes.

However there is a bigger issue.

If you search RFM69.h for 'static' you will see many of the key attributes e.g. 'DATA' are declared static. This means they will be shared between both instances, clearly unacceptable. To fix this

• The static keyword wil have to be removed from the attributes in the header file.
• The declarations of the same variables in the cpp will have to be commented out.

I am not sure of the reason for these being static was, or if making this change will have a bigger impact, clearly it will need to be tested.

Kiwi

[donaldhwong]

Dear KiwiSinceBirth,

Thank you so much for helping.  I understand what you are saying.  When I compile the code, I got the following errors.  Can you continue to help?  Very much appreciated.

C:\Users\jmjm\Documents\Arduino\libraries\RFM69\RFM69_original.cpp:44: error: 'RFM69* RFM69::selfPointer' is not a static member of 'class RFM69'
C:\Users\jmjm\Documents\Arduino\libraries\RFM69\RFM69_original.cpp: In member function 'bool RFM69::initialize(byte, byte, byte)':
C:\Users\jmjm\Documents\Arduino\libraries\RFM69\RFM69_original.cpp:107: error: 'selfPointer' was not declared in this scope
C:\Users\jmjm\Documents\Arduino\libraries\RFM69\RFM69_original.cpp:112: error: argument of type 'void (RFM69:()' does not match 'void (*)()'
C:\Users\jmjm\Documents\Arduino\libraries\RFM69\RFM69_original.cpp: In static member function 'static void RFM69::isr0()':
C:\Users\jmjm\Documents\Arduino\libraries\RFM69\RFM69_original.cpp:330: error: 'selfPointer' was not declared in this scope

[kiwisincebirth]

the errors would indicate the selfPointer needs to be declared 'static', as per the code I posted.

[donaldhwong]

Hi Again,

I was using your code.  And I checked, they were declared as static.  Weird..

Thank you for you quick response.

Donald

[kiwisincebirth]

Strange. I didn't test the code I posted, but it did compile, I used one of the example scetches straight from felix's library to test this. Question: I just noticed in your error list it says RFM69_original.cpp. Is this the right file?

[donaldhwong]

Thank you KiwiSinceBirth.  You are my life saver.  I have successfully compiled it and run Felix's Gateway code under Arduino IDE.

Will try to run both RFM69W simultaneously after finish my shopping for Christmass gift today.

Thank you again.
Donald

[donaldhwong]

Hi KiwiSinceBirth,

I tried both radio(10,2,false,0); and radio(7,3,false,1) in two separate instance with your edits. Both modules work well.  One without antenna showed -60 RSSI and the one with the anntenna showed 43 RSSI. 

Then I defined radio(10,2,false,0); and radio2(7,3,fasle,1); and radio2.initialize(FREQUENCY,3,NETWORKID);  without any additional command.  The default radio still works fine.

But as soon as I inserted "if radio2.receivedDone(){}"  The interrupt stop working.  I get only one line below at the Serial Monitor:

Listening at 433 Mhz...
SPI Flash Init OK. Unique MAC = [D7:64:44:24:63:5F:71:31:]
#[1][2] FLASH_MEM_ID:0xEF30   [RX_RSSI:-41] - ACK sent. Pinging node 2 - ACK...ok!

That's it. 

Please advise what else I needed to do.

Thank you for your continual support.

[donaldhwong]

Hi KiwiSinceBirth,

What I found out just now was both RFM69W were initialized with current code.  I suspect that the second RFM69w interrupted only once, because I print out the radio2.readRSSI() after each radio.receiveDone().  I only got the same number.

[kiwisincebirth]

Just to be clear you removed the static keyword from the following definitions in RFM69.h ? and commented the similar declarations in the cpp file.

Code Select Expand

    volatile byte DATA[RF69_MAX_DATA_LEN];          // recv/xmit buf, including hdr & crc bytes
    volatile byte DATALEN;
    volatile byte SENDERID;
    volatile byte TARGETID; //should match _address
    volatile byte PAYLOADLEN;
    volatile byte ACK_REQUESTED;
    volatile byte ACK_RECEIVED; /// Should be polled immediately after sending a packet with ACK request
    volatile int RSSI; //most accurate RSSI during reception (closest to the reception)
    volatile byte _mode; //should be protected?

[donaldhwong]

Hi KiwiSinceBrith,

Nop, sorry. I thought you were trying to explain what you did, instead of an assignment for me.  :-)

Now I did that.  radio2.receiveDone() no longer hang up the program, or stop radio1 from functioning in the loop, but it does not interrupt either.

I commented out "radio1.receiveDone()" and leave only "radio2.receiveDone()".  It does not work.  I then commented out

/*   
  if (_interruptNum==0) {   
   //detachInterrupt(1);
   attachInterrupt(_interruptNum, RFM69::isr0, RISING);
     selfPointer0 = this;
  }
  */

and leave only the

attachInterrupt(1, RFM69::isr1, RISING);
selfPointer1 = this;

still no trigger.

But, if I reverse radio_1 to the second module and radio_2 to the first/embedded module, it is still radio_1 that triggers.  So problem is not in the hardware.  It is in the library. My guess.

Please advise. 

[kiwisincebirth]

At this point it would be very hard to work out why this isn't working. The advice so far was how to get the class defined in such a way that two instances are running separately, and receiving interrupts correctly.

A couple of possibilities

• While one instance is interacting with the radio (and disabling interrupts) the other instance doesn't get the interrupt that it needs.
• While one instance is interacting with the radio (calling writereg() ) the other instance gets and interrupt and takes over SPI, when returned from the interrupt the first radio could have timed out.

Another words a race condition, on interrupts and the shared SPI bus. And this is when it gets difficult as the existing library wasn't designed to work this way. Really you are going to have to do a bit of debugging to see where and why things stop working. Primarily the code needs to isolate the use of the SPI bus and handle interrupts with as little impact to the other radio, but again very hard to know.

[donaldhwong]

Hi KiwiSinceBirth,

I understand what you are saying and I also suspected that.  Especially when the messages are being sent so fast.

Is there a way that I could Wait for Interrupt 0 and only 0, ignore all other interrupt, process that.  Then wait for Interrupt 1, process that, while ignore all other interrupt.  Just alternating waiting for 0 and 1 and 0 and 1? 

Because for my application, it does not need to be real time.  The project can afford to loose some signals.

Please advise.  Thank you for your continual help.

[kiwisincebirth]

I would be possible to modify the code, there are a couple of things of note:

The select() and unselect() functions are responsible for the SPI CS enable and disable, and are placed around use of SPI communications. These functions also disable interrupts, preventing other code from interfering, while in progress.

Currently this happens in the low level functions (e.g. writeReg() and readReg()), but a higher level function could call many low level functions to perform one task. If these select() and unselect() were called in the high level functions, then the exclusivity would be broadened, and potentially block interrupts happening during a single user call to the library.

Another issue which could be a problem is interruptHandler() itself. In the first line of code it calls readReg(), which then calls select() and unselect(). The unselect() method enables interrupts, so potentially from that point forward the interrupthandler() could itself be interrupted. A first step may be to code the interrupt handler so that it never enables or disables interrupts.

My understanding is that interrupts are implicitly disabled before the interrupt handler is called, and then re-enabled when the handler is finished.

Kiwi

[donaldhwong]

Thank you, KiwiSinceBirth,

Got it.  I will give it a try. 

I have another idea.  Since there are success in implementing one RFM12B with one RFM69W working as a bridge, what if we create a second class, say RFM69_1. 

What do you think?  If you believe it worth a try, then what do I need to pay attention to?

Thank you kindly.
Donald

[kiwisincebirth]

Answering your question about two classes, Yes it is worth a try but my guess is it won't make too much difference, since we have removed all static (shared) data, except of course the interrupt handler.

Another possibility is 'SRAM' is getting low, here is an article that describes problem, and provides solution https://learn.adafruit.com/memories-of-an-arduino/measuring-free-memory

ALSO

After my last post I did some work and implemented (compile only) what I stated in My post.

The change is centred around the select() and unselect() functions. These functions perform what I call 'reference counting' which means the first call to select() does the SPI enable and interrupt disable. Then any further call to select() is ignored, except to increment a counter. Then when unselect() is called the counter is decremented, and on the final unselect() when counter reaches zero, the SPI is disabled and interrupts enabled.

This means select() and unselect() can now be placed anywhere in code, so long as they are in matching pairs.

There is only two slight issues millis() and Serial.print(). These functions need interrupts to be functioning, so I have omitted select() and unselect() around code that uses these functions, or code that calls code that uses these functions. This happens quite a bit e.g waiting for an ACK response, where the response is received and handled by the interrupt routine itself. Clearly this sort of code cannot use select() unselect(), as would prevent interrupts from occurring.

I have also fixed interruptHandler(). The first line of code is select(true); the boolean flag tells select() and unselect() to NOT enable/disable interrupts, which (as stated previously) could have been an issue.

receiveDone() specifically sets and clears interrupts. I have left it alone, except to make the enable interrupts explicit, and ensured that any code that calls this function itself has not called select() unselect(), which could have caused a conflict.

Anyway give it a try.

Kiwi