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

ng_slip: initial import #2688

Merged
merged 6 commits into from
May 23, 2015
Merged

ng_slip: initial import #2688

merged 6 commits into from
May 23, 2015

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Mar 22, 2015

Updated version of #1454. I removed the board_uart0 support for now, to keep it simple.

Depends on #2575. (merged)

Needs #2901 to build properly (merged)

@miri64 miri64 added Area: network Area: Networking Type: new feature The issue requests / The PR implemements a new feature for RIOT NSTF labels Mar 22, 2015
@miri64 miri64 force-pushed the ng_slip/feat/initial branch from c402c53 to b0b0e6f Compare March 23, 2015 22:21
break;

default:
if (uart_write(uart, data[i]) < 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will not work (you will lose chars). Use uart_write_blocking() when you use the uart like this. Better (and I really would to this here): use the transmit interrupt and a transmit buffer!

static ringbuffer_t *out_buf;

_slip_send(..) {
    ...
    default:
    ringbuffer_add_one(&out_buf, data[i]);
    uart_tx_begin(uart);
    ...
}

// and
int _slip_rx_cb(void *arg) {
    if (_out_buf.avail > 0) {
        char c = (char)ringbuffer_get_one(&out_buf);
        uart_write(uart, c);
        return 1;
    }
    return 0;
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you mean tx_cb, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem I have with this is, that we than need a second buffer of at least size IPV6_MTU.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and true for the buffer size. But the blocking use of the UART interface is very slow. This basically means, that for sending one full IPv6 MTU, you controller basically blocks for about 10ms...

@haukepetersen
Copy link
Contributor

You should spend the SLIP interface a device descriptor to make it possible to run more than one instance at a time! This could be interesting for setups, where one interface is connected to linux, and another one is connected to another RIOT based board - some of our boards have up to 8 UART interfaces :-)

@miri64
Copy link
Member Author

miri64 commented Mar 23, 2015

I was thinking about it, but than thought it might be an interesting challenge for future generations :D

@miri64 miri64 force-pushed the ng_slip/feat/initial branch from c4d8ef0 to 2c84dee Compare March 24, 2015 00:14
@haukepetersen
Copy link
Contributor

hm, why not do it right in the first place? So others don't even get the idea to implement drivers with global variables (and the SLIP device would be in line with all new readio and device drivers...).

@miri64
Copy link
Member Author

miri64 commented Mar 24, 2015

You saw that I did it, right?

@haukepetersen
Copy link
Contributor

Ups, did not. But the new version does not work -> you will have data garbage as to shared uart buffers... I would really do it the same way we do any other device driver: declare the device descriptor in the header file and include the uart buffers with full size in this struct. Then it is up to the user of a slip device to allocate this device descriptor (not on the stack!) and just give the pointer to this memory to the init function.

@miri64
Copy link
Member Author

miri64 commented Mar 24, 2015

The uart buffers are not shared, they are uart local, but I can make the struct public. As for the full size data buffers: I chose very carefully not to include the size of the ringbuffers into this PR, simply because it mainly depends on the network layer what size is required.

@miri64
Copy link
Member Author

miri64 commented Mar 24, 2015

Done

@miri64 miri64 force-pushed the ng_slip/feat/initial branch from a78734a to 14e48f3 Compare March 24, 2015 17:11
@miri64
Copy link
Member Author

miri64 commented Mar 24, 2015

Sorry, somehow there was a foreign commit in my rebase

@miri64
Copy link
Member Author

miri64 commented Mar 31, 2015

Rebased to current master

@OlegHahm OlegHahm force-pushed the master branch 2 times, most recently from 9f184dd to 45554bf Compare March 31, 2015 13:01
@miri64 miri64 added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Mar 31, 2015
@miri64 miri64 force-pushed the ng_slip/feat/initial branch from e225b57 to 6cf1e6f Compare April 16, 2015 16:12
@miri64
Copy link
Member Author

miri64 commented Apr 16, 2015

Rebased to current master

@miri64 miri64 force-pushed the ng_slip/feat/initial branch 2 times, most recently from 465fc8a to 5ec27fd Compare April 24, 2015 16:36
@miri64
Copy link
Member Author

miri64 commented Apr 24, 2015

Fixed a whitespace error

@miri64 miri64 force-pushed the ng_slip/feat/initial branch from 5ec27fd to 301643d Compare April 27, 2015 07:38
@miri64
Copy link
Member Author

miri64 commented Apr 27, 2015

Rebased to current master

@miri64 miri64 force-pushed the ng_slip/feat/initial branch from eaefaf6 to 83d0252 Compare May 22, 2015 09:51
@miri64
Copy link
Member Author

miri64 commented May 22, 2015

Rebased to current master and #3005.

@miri64 miri64 force-pushed the ng_slip/feat/initial branch from 83d0252 to 6d556f7 Compare May 22, 2015 10:03
@miri64
Copy link
Member Author

miri64 commented May 22, 2015

Another day another rebase :D

@miri64 miri64 force-pushed the ng_slip/feat/initial branch 2 times, most recently from 24f1c27 to efe0f1f Compare May 23, 2015 09:56
@miri64
Copy link
Member Author

miri64 commented May 23, 2015

Rebased to current master… can we FINALLY merge this (this still needs a re-ACK) (no it doesn't)

@miri64 miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label May 23, 2015
@miri64
Copy link
Member Author

miri64 commented May 23, 2015

Travis fails in unrelated applications. What's happening oO @phiros @OlegHahm @x3ro @gebart?

@miri64 miri64 force-pushed the ng_slip/feat/initial branch from efe0f1f to f94fa7a Compare May 23, 2015 11:20
@miri64
Copy link
Member Author

miri64 commented May 23, 2015

Ah, a rebase error was the culprit. Let's see if it works now.

@miri64 miri64 removed the State: waiting for other PR State: The PR requires another PR to be merged first label May 23, 2015
@miri64 miri64 force-pushed the ng_slip/feat/initial branch from f94fa7a to 3fa750d Compare May 23, 2015 11:41
@miri64
Copy link
Member Author

miri64 commented May 23, 2015

This PR still fails due to the license checker… I added a pattern file which is to my knowledge correct, but it still fails… @LudwigOrtmann @OlegHahm @mehlis help!

@miri64 miri64 force-pushed the ng_slip/feat/initial branch from 3fa750d to d2275ae Compare May 23, 2015 11:47
@miri64
Copy link
Member Author

miri64 commented May 23, 2015

Okay… don't know exactly why, but it seems to be fixed... Now cppcheck complaining about the Contiki code… do I have to fix those?

@jnohlgard
Copy link
Member

Ignore the cppcheck warnings/errors in tunslip6 (Contiki). We'll fix it in a follow up PR.

@miri64
Copy link
Member Author

miri64 commented May 23, 2015

Ok, then I'll merge

miri64 added a commit that referenced this pull request May 23, 2015
@miri64 miri64 merged commit 1d74a73 into RIOT-OS:master May 23, 2015
@miri64 miri64 deleted the ng_slip/feat/initial branch May 23, 2015 19:19
@miri64
Copy link
Member Author

miri64 commented May 23, 2015

\o/

typedef struct xbee_params {
uart_t uart; /**< UART interfaced the device is connected to */
uint32_t baudrate; /**< baudrate to use */
} xbee_params_t;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry to being a bit late: but this is copy pasta, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes... Should I open a PR or did you already?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, I didn't, so feel free.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants