Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add feature to have no DIO connection #34

Closed
wants to merge 4 commits into from

Conversation

hallard
Copy link

@hallard hallard commented Aug 10, 2016

Software feature to remove needing DIO connections if needed in case you're restricted in GPIO available, you can avoid using any GPIO connection.
To activate this feature, you just need to declare 3 .dio to LMIC_UNUSED_PIN as follow for example to WeMos Lora Shield

// Example with NO DIO pin connected
const lmic_pinmap lmic_pins = {
    .nss = 16,
    .rxtx = LMIC_UNUSED_PIN,
    .rst = LMIC_UNUSED_PIN,
    .dio = {LMIC_UNUSED_PIN, LMIC_UNUSED_PIN, LMIC_UNUSED_PIN},
};

@matthijskooijman
Copy link
Owner

matthijskooijman commented Aug 10, 2016

I had a quick glance over the code, it looks ok. However, I'm afraid that polling SPI on every loop will be too much overhead. I think that now it even polls the SPI registers up to three times in a single loop, which doesn't really add anything, once should be enough.

Ideally, LMIC should keep track of what DIO pins it expects an event on, so the HAL can only poll SPI when an event is pending that cannot be read through a pin (this would help even when all pins are connected, since then there is no need to poll any pins if no event is pending. This helps because digitalRead() isn't exactly very fast either).

This probably requires invasive changes to the code, so that needs more thought. Not sure if there is a good intermediate step for this, though.

@hallard
Copy link
Author

hallard commented Aug 11, 2016

@matthijskooijman, thanks having time to look into.

I agree this is not the ideal solution but for those who don't have enough GPIO pins, and on ESP8266 as soon as your using SPI, it's really limited, so 3 pins for IRQ is just too much. In this case pooling is working fine. In any case, in the code I made go into reading IRQs using SPI only once per loop (look below, i=NUM_DIO make exit the for loop) because as soon you get in, we're reading all IRQs register (as original code) and we exit without getting other IRQ until next loop (because we've just done all the job). And if one fire before we ended, it will be managed on next loop.

 for (i = 0; i < NUM_DIO; ++i) {
        if (lmic_pins.dio[i] == LMIC_UNUSED_PIN)
        {
            // Check IRQ flags in radio module
            if ( radio_has_irq() ) 
                radio_irq_handler(0);

            // We've soft checked IRQ reading Radio register no need to continue
            // Setting this will exit us from for loop
            i = NUM_DIO;
        } else {
            if (dio_states[i] != digitalRead(lmic_pins.dio[i])) {
                dio_states[i] = !dio_states[i];
                if (dio_states[i])
                    radio_irq_handler(i);
            }
        }
    }

By the way current implementation pool input with digital read 3 times a loop (3 lines) so I think we're not so , would be interesting looking time used to read SPI IRQ register, I'll take a look on this ASAP.

But for sure idea will be doing nothing until we're fired by a real IRQ line and associated code vector, but this will be more tricky to implement because depending on target it's not the same and for example a ATMega328 as just 2 reals IRQ hardware lines.

@hallard
Copy link
Author

hallard commented Aug 11, 2016

I just changed DIO implemention on hal.c, setting a flag in hal_io_init() when we're not using DIO (detected and set if all DIO line in structure setting are set to LIMC_UNUSED_PIN)
then in hal_io_check() flag is tested and no for loop if no DIO used. I think it will not change really something but may be you'll find the code much clean. Just let me know if you want me doing something else or change anything

static void hal_io_check() {
    // We have DIO line connected?
    if (check_dio == 1) {
        uint8_t i;
        for (i = 0; i < NUM_DIO; ++i) {
            if (dio_states[i] != digitalRead(lmic_pins.dio[i])) {
                dio_states[i] = !dio_states[i];
                if (dio_states[i]) {
                    radio_irq_handler(i);
                }
            }
        }
    } else {
        // Check IRQ flags in radio module
        if ( radio_has_irq() ) 
            radio_irq_handler(0);
    }
}

@hallard
Copy link
Author

hallard commented Aug 11, 2016

@matthijskooijman
Just done some testing about overhead in hal_io_check() for both methods:

  • Arduino Zero with DIO enabled (just check pin input) => 7µs
  • Arduino Zero with No DIO (read RFM95 SPI IQR register) => 47µs
  • ESP8266@160MHz with No DIO (read RFM95 SPI IQR register) => 17µs

I can't test DIO on ESP8266 because the board I have have no DIO connection to GPIO pins.

Do you think 47µs could impact LoraWAN stack timings, because seems to works fine on my side even on Zero ?

@matthijskooijman
Copy link
Owner

Do you think 47µs could impact LoraWAN stack timings, because seems to works fine on my side even on Zero ?

I think 47us is significant and ideally it would be optimized further, but for now it seems fair enough. I am curious how much it takes on an AVR chip, and how much longer when the SPI speed is lowered to 1Mhz (as proposed on another PR). Do you have an AVR handy? If not, perhaps you could add your time-printing code to this PR in a separate commit (or even a completely different branch in your repo), then I can try running it as well.

The approach of setting a flag on initializaton sounds like a sane approach to me. I haven't looked at the code yet and this will probably be a few weeks until I'll find the time, but thanks already for your work!

@matthijskooijman
Copy link
Owner

Oh, and for your actual question, I think there is at least 1280us of leeway in the the timing of the RX windows (see the "Timing" section of the README), so 47us shouldn't hurt the LoRaWAN reception by itself.

@hallard
Copy link
Author

hallard commented Aug 11, 2016

So, I've done some testing, here the results

  • Atmega328P@16MHz with DIO enabled (just check pin input) => 20µs
  • Atmega328P@16MHz SPI@10MHz with No DIO (read RFM95 SPI IQR register) => 45µs
  • Atmega328P@16MHz SPI@4MHz with No DIO (read RFM95 SPI IQR register) => 50µs
  • Atmega328P@16MHz SPI@1MHz with No DIO (read RFM95 SPI IQR register) => 80µs

That's interesting, as you can see the impact of DIO/NO DIO is "just" twice the time on 328p ;-)

From my point of view, doing OR with 3 diodes and using only one input line is the best approach, only one pin to check on DIO loop and no need to talk with SPI if not needed, and cherry on the cake, only one GPIO used!
To check this and for fun, here is the result

  • Atmega328P@16MHz SPI@10MHz with ONLY ONE DIO => 8µs

And now, of course attach an hardware interrupt to this DIO line and we go to 0 in loop ;-)

@Oliv4945
Copy link
Contributor

Thank you for those figures ! Keep in mind that lot of atmega328 are used battery powered at 3.3V/8 MHz, so more overhead :)
The solution with diode in OR gate is the one I use on my nodes, and it works well !

@gizmocuz
Copy link

Interesting... are you using also a pull-down resistor like in:
image

@zoutepopcorn
Copy link

Very very cool 👯 .

Getting the error FAILURE, radio.c 692
#ifdef CFG_sx1276_radio
ASSERT(v == 0x12 );

Before everything was working.

I connected the SPI and SS. And used LMIC_UNUSED_PIN. I will look at it later

@hallard
Copy link
Author

hallard commented Aug 12, 2016

@gizmocuz
Yes exactly this one schematic I'm using on WeMos Lora shield GPIO15 has a pull down so it's not present on the schematic.

@zoutepopcorn
The problem you have is that your device is not detected (wiring problem or wrong device type), but nothing related to DIO pin feature. I had the same yesterday testing ATMega328, I checked all DIO of my breakout 3 times until I decided to change my arduino board, then all went fine, may be SPI pin have a solder problem on my custom board, I'll check it this later ;-)

@hallard
Copy link
Author

hallard commented Aug 22, 2016

Hi @matthijskooijman
Any change this option may be merged to master branch?
I'm asking this because I've made new modifications so the lib works on Raspberry PI with samples code and some other goodies but the RPI code is based on this PR. So I won't be able to PR the one for RPI without this one and if I push the RPI one on my git now, this PR will inherit of the new RPI modification and I would prefer having both PR (DIO and RPI) dissociated.

But may be I'm doing wrong, I'm not super confident with git, branch and all git command line stuff (I'm mainly doing PR and commits with Git GUI from my Windows Machine) especially when I got a repo forked that need to be synced with master remote (you one).

@matthijskooijman
Copy link
Owner

@hallard, I haven't had time to look closely at your PR yet, and probably won't have time for it this week. Your understanding of git is correct, if you have another PR dependent on this one, this one's commits will be included. I haven't found any way yet to "hide" the redundant commits from such a PR. However, you could just submit it anyway, so we can at least review the code.

@brainstorm
Copy link

brainstorm commented Apr 21, 2017

@hallard @matthijskooijman This worked great on my two ESP8266's+RFM95's: one perfboard gateway running things4u's gw software while the other smaller custom board, acting as client with arduino-lmic's ABP example on TTN.

So thanks @hallard, you saved my board since I did run out of GPIO's to do what I wanted to do in my project 👍

I'm a bit concerned about the node id's my gateways sees though, not sure they should be random given that I'm sending those from the very same node (pretty sure it's noone else on indoors reach on my same SF and channel):

image

My perfboard gateway does use DIO0 and DIO1, so happy to run some benchmarks if you show me how you did that @hallard ;)

EDIT: Those nodes are indeed other devices (including 3 packets from mine) :-! Seems that the gateway's HOPping mechanisms do not work so well, so fixing it to a specific SF and channel allows it to see devices, cool!

Also, I would be super happy to see this merged ;)

@1993aicha
Copy link

hi @hallard
I worked with your bib for raspb and it is working (Thank u),
but i was wondering how can i change code to get my interruptions on DIOs again.

@karlTGA
Copy link

karlTGA commented Jan 19, 2019

hi @hallard,
thanks for that feature. I build your wemos shield and it works very well. Now I want to use deep sleep on the esp and have to connect the GPIO16 with RST, which is easy thanks to the jumper. But is that with this fork possible. If yes, what lmic_pinmap is to set. I tried for NSS LMIC_UNUSED_PIN and some other pins. But LMIC_UNUSED_PIN leads to a failure in hal.cpp:24 and other pins to a failure in lmic/radio.c:692.

@ggaben1
Copy link

ggaben1 commented Apr 7, 2019

hi @hallard,
thanks for that feature. I build your wemos shield and it works very well. Now I want to use deep sleep on the esp and have to connect the GPIO16 with RST, which is easy thanks to the jumper. But is that with this fork possible. If yes, what lmic_pinmap is to set. I tried for NSS LMIC_UNUSED_PIN and some other pins. But LMIC_UNUSED_PIN leads to a failure in hal.cpp:24 and other pins to a failure in lmic/radio.c:692.

Hi karlTGA
It seems to me the same thing. Have you ever found a solution?

thx
Gaben

@hallard
Copy link
Author

hallard commented Apr 17, 2019

The error is happening at compile time or when the program is running?

  • step one, add solder pad (blue) you may have done it
  • step two, cut the trace (red) between GPIO and SEL to leave NSS pin grounded by the pullup and not connected to RST may be this one you issue, and yes NSS LMIC_UNUSED_PIN is the correct option

image

Schematic for reference of what's need to be done
image

@ggaben1
Copy link

ggaben1 commented Apr 17, 2019

The error is happening at compile time or when the program is running?

step one, add solder pad (blue) you may have done it
step two, cut the trace (red) between GPIO and SEL to leave NSS pin grounded by the pullup and not connected to RST may be this one you issue, and yes NSS LMIC_UNUSED_PIN is the correct option

Schematic for reference of what's need to be done

Hello Hallard,
this happens on run. There is no problem with translation. The wiring was done exactly as recommended. Maybe the nss pin is absolutely necessary for timing with SPI communication?
Was this solution working for you?

thx
Gaben

@hallard
Copy link
Author

hallard commented Apr 17, 2019

NSS is to tell device "I select you, you need to answer to me" and it's active LOW. It's used when you have multiple SPÏ slave on same SPI bus, you select the one you're talking to.

According the datasheet pulling to low with R3 (is it soldered?) should does the trick and select always.
What's the voltage on NSS pin? I did not tried on this board and this chip, but already tried on other SPI device without any problem, that's strange.

image

Another option, I saw RST pin of the RFM95 is connected to RST of the Wemos, could you try to cut the RST trace going to RFM95 to see or add a small delay on startup to let RFM95 going out of reset?

image

@ggaben1
Copy link

ggaben1 commented Apr 17, 2019

NSS is to tell device "I select you, you need to answer to me" and it's active LOW. It's used when you have multiple SPÏ slave on same SPI bus, you select the one you're talking to.

According the datasheet pulling to low with R3 (is it soldered?) should does the trick and select always.
What's the voltage on NSS pin? I did not tried on this board and this chip, but already tried on other SPI device without any problem, that's strange.

image

Another option, I saw RST pin of the RFM95 is connected to RST of the Wemos, could you try to cut the RST trace going to RFM95 to see or add a small delay on startup to let RFM95 going out of reset?

image

I understand and you may be right, but according to documentation "A transfer is always started by the NSS pin going low and the frame ends when NSS goes high. Perhaps this is not relevant, but in any case I get an error if the nss pin is connected to ground with a 100K Ohm resistor. Who would you possibly try to do that for you? Maybe I'm doing something wrong ...
Thx
Gaben

@ggaben1
Copy link

ggaben1 commented Apr 17, 2019

I've checked it again. With recommended wiring, nss pin config: LMIC_UNUSED_PIN, but error:
20: 04: 58.151 -> Starting
20: 04: 58.184 -> FAILURE
20: 04: 58.184 -> C: Users\Gabor\Documents Arduino\libraries\arduino-lmic-1.5.0-arduino-2 cc

Maybe the address is wrong?

"ASSERT(v == 0x12 );"

@chaveiro
Copy link

If this contribution is optional and users have tested it with success for 3 years, why was it not merged yet ?

@matthijskooijman
Copy link
Owner

To be honest, I just haven't looked in enough detail yet. Also, I've been a bit hesitant to make changes to this Arduino-specific version (initially because there was some upstream development, later because I considered restarting from a less Arduino-specific repository, but that never got finished). In the meanwhile, it seems that the MCCI fork of this library has done a lot more development, so I suspect that that might a better starting point for developments like these...

I should probably put up a notice somewhere and maybe make this official, but I wanted to have a closer look at the MCCI version first and I have not found the time for that yet...

@IoTThinks
Copy link

To be honest, I just haven't looked in enough detail yet. Also, I've been a bit hesitant to make changes to this Arduino-specific version (initially because there was some upstream development, later because I considered restarting from a less Arduino-specific repository, but that never got finished). In the meanwhile, it seems that the MCCI fork of this library has done a lot more development, so I suspect that that might a better starting point for developments like these...

I should probably put up a notice somewhere and maybe make this official, but I wanted to have a closer look at the MCCI version first and I have not found the time for that yet...

Hi,
I try to compile the code for no DIO in my ESP32
I hit "hal.cpp:81:28: error: 'radio_has_irq' was not declared in this scope"

Is this radio_has_irq only for Arduino board?
Or I miss something else.

Thanks a lot.

@matthijskooijman
Copy link
Owner

This repository is now deprecated, see #297 for some more background. I'm grateful for your contribution, but it will no longer be merged. I'm recommending people to use the MCCI version of LMIC instead. If this PR addresses an issue that also exists in that version, I would encourage you to resubmit your contribution there, so it might benefit other users. I'm sorry for the inconvenience...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.