Skip to content

PPP packets from Serial on interrupt basis #7646

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

Open
3 of 6 tasks
FIAV1 opened this issue Oct 8, 2020 · 11 comments
Open
3 of 6 tasks

PPP packets from Serial on interrupt basis #7646

FIAV1 opened this issue Oct 8, 2020 · 11 comments

Comments

@FIAV1
Copy link

FIAV1 commented Oct 8, 2020

Basic Infos

  • This issue complies with the issue POLICY doc.
  • I have read the documentation at readthedocs and the issue is not addressed there.
  • I have tested that the issue is present in current master branch (aka latest git).
  • I have searched the issue tracker for a similar issue.
  • If there is a stack dump, I have decoded it.
  • I have filled out all fields below.

Platform

Settings in IDE

  • Module: Generic ESP8266 Module
  • Flash Mode: qio
  • Flash Size: 4MB
  • lwip Variant: v2 Higher Bandwidth
  • Reset Method: ck
  • Flash Frequency: 40Mhz
  • CPU Frequency: 80Mhz
  • Upload Using: SERIAL
  • Upload Speed: 115200

Problem Description

I talked with @d-a-v about implementing a mechanism using interrupts to handle data coming from Serial and send it directly to ppp_input() function. Can someone point me to the right direction?

@Tech-TX
Copy link
Contributor

Tech-TX commented Oct 10, 2020

David may have meant to send you to the gitter, https://gitter.im/esp8266/Arduino

@d-a-v
Copy link
Collaborator

d-a-v commented Oct 12, 2020

It is possible to do that.
The interrupt handler in uart.cpp with minor changes can trigger a user scheduled function. You would add your PPP handler.

@FIAV1
Copy link
Author

FIAV1 commented Oct 12, 2020

Nice, thanks @d-a-v !
The user scheduled function you are talking about, is a callback function or is it set in another way?
Could you give me an example to start with?

@d-a-v
Copy link
Collaborator

d-a-v commented Oct 12, 2020

Scheduled functions are registered using Schedule.h API.

Uart ISR must be modified to trigger interrupt from the first byte (INTRIGG is currently 16).
A mechanism should prevent to register a callback when/until the previous trigger wasn't executed.

@d-a-v
Copy link
Collaborator

d-a-v commented Oct 15, 2020

(edited)

A recurrent scheduled proposal is in #6680, as transparent background transfer in CONT stack (user stack).
Incoming check is done at most every millisecond.

It may be a bit too fast and not adapted with serial link speed. Maybe using the serial irq would be more efficient. Indeed a PPP packet is 8 bytes length at minimum, the potential Serial interrupt API could be adapted to take this kind of input into account. Also bypassing the HardwareSerial class by making another Serial class, not buffering but based only on interrupt might me more efficient for that kind of use-case (let's consider #6680 first?).

However as said in gitter by @devyte, doing this in a user loop() will work (that was tested with SoftwareSerial as PPP feeder).

@FIAV1
Copy link
Author

FIAV1 commented Oct 19, 2020

I could try to modify my sketch in order to try the mechanism with the user loop() and see how it goes.

Unfortunately, I'm not that good at writing such low level code; I could give it a try, but I can't guarantee my work will be that good.

@d-a-v
Copy link
Collaborator

d-a-v commented Oct 21, 2020

In your main loop:

    size_t avail = Serial.available();
    if ((avail > 0)
    {
        if (avail > BIG_ENOUGH)
            avail = BIG_ENOUGH; // in stack, not more than 128
        // XXX block peeking would be useful here
        char mybuf [avail];
        avail = Serial.readBytes(mybuf, avail);
        pppos_input(ppp, mybuf, avail);
    }

@FIAV1
Copy link
Author

FIAV1 commented Nov 18, 2020

After a month of tests, here's my conclusion:

  • Handle packets on interrupt basis create many problems, all tied to the fact that it's impossible to know the packet incoming rate, so the risk is that the ppp input function is called too many times, lowering performance (or the opposite case)
  • Handle packets by using the handler function on the main loop create problems too (for example, all connection tries for tcp sockets will fail, since all the functions in the main loop will be paused when calling connect())
  • At the moment the best solution is using the handler function with the schedule_recurrent_function() function, setting the timing to e.g. 1000us and using a big enough space for uart's fifo virtual memory (i set it to 3000 bytes)

@d-a-v
Copy link
Collaborator

d-a-v commented Nov 18, 2020

Thanks for the report !

By rereading the posts above, I should have realized that the main loop solution couldn't work because of the second reason you just gave. Sorry for that. A similar issue was hapenning with ethernet and a mqtt library stuck in its connecting loop. That was initially what led me to add recurrent scheduled functions, for network management in background in CONT. Glad to know it's also efficient with PPP.

Still, I'm wondering if a recurrent scheduled function returning false and installed by serial interrupt would not be the most efficient. Did you try that ? Remember that a simple scheduled functions is executed not more often that once at every loop, while a recurrent one returning false (= executed once like a not-recurrent one) can also be triggered at yield()s (= during the connecting loop).

@FIAV1
Copy link
Author

FIAV1 commented Nov 18, 2020

I didn't try it yet. The problem is that I'm used to program at higher levels, so it's a bit difficult for me to put hands on that part of code without having some sort of guide or explanation about it... I think it would take a lot of time only to understand what the code is doing, then by the time I try to change something, version 12 of this sdk would have already come out 🤣

@d-a-v
Copy link
Collaborator

d-a-v commented Nov 18, 2020

It's point 1:

Handle packets on interrupt basis create many problems, all tied to the fact that it's impossible to know the packet incoming rate, so the risk is that the ppp input function is called too many times, lowering performance (or the opposite case)

but instead of handling packet, you need to defer handling in a recurrent scheduled function like this:

IRAM_ATTR void isr ()
{
    // handle_ppp_packets() will be called once at next yield()/delay()/loop()
    schedule_recurrent_function_us(0, [](){ handle_ppp_packets(); return false; });
}

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

No branches or pull requests

3 participants