Skip to content

sys: net: Initial import of a general interface to a network protocol#1448

Merged
miri64 merged 1 commit intoRIOT-OS:masterfrom
miri64:network-layer-interface
Nov 20, 2014
Merged

sys: net: Initial import of a general interface to a network protocol#1448
miri64 merged 1 commit intoRIOT-OS:masterfrom
miri64:network-layer-interface

Conversation

@miri64
Copy link
Copy Markdown
Member

@miri64 miri64 commented Jul 18, 2014

The definitions for the new network stack API

@miri64 miri64 added this to the Release NEXT MAJOR milestone Jul 18, 2014
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Jul 19, 2014

@thomaseichinger @rousselk could you maybe look if the dummy MAC layer makes sense somehow. I based it mainly on #925, (edit ->) but tried to generalize the assumptions a bit.

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Jul 28, 2014

@LudwigOrtmann since HEAD~1 touches nativenet, this PR might interest you, too.

@thomaseichinger
Copy link
Copy Markdown
Member

The dummy_mac seems good to me. One thing we have to consider, what if we have 2+ radio devices of the same kind e.g. same radio driver implementation. We'd have to determine somehow which device to use. MAC / radio driver internally we can talk about either passing the device to each function call, what we do in low level drivers right now, or save the device in a variable. I'd plead for version 1, since it is more consistent to our existing model.
Anyway, I think we need a radio_device_t or similar in dummy_mac_init like we use for UART, GPIO and SPI.

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Jul 28, 2014

The idea for dummy_mac here is very simple: the radio driver is "identified" by it's functions, which the driver developer supplies to dummy_mac via dummy_mac_init() in ops (yes, a few driver functionalities need to be adjusted/expanded). The call of dummy_mac_init() can than be made on board (or maybe better driver) initialization, if no better suitable MAC layer is available.
I took great care that it is possible to call dummy_mac_init() with a whole different set of operations, so that you can have 2+ separate instances of the dummy_mac control thread; theoretically one for each radio/cable.

Since radio devices are very heterogenous and boards can have reattachable network devices (:point_right: arduino) I think there is no easy way via some radio_device_t as we have for example with UART, GPIO, etc. On the contrary: since you have to check for the value of radio_device_t on every command receival, the code might get more complicated than now.

@thomaseichinger
Copy link
Copy Markdown
Member

I agree with you but I don't think we can find a generic way around radio_device_t. It is used then for the underling layers (mainly the radio driver functions provided in ops) to determine which SPI/I2C -> GPIOs -> hardware radio module to use. These are all specified in the board's periph_conf.h but I think there has to be a initial hint to determine this. Except we assume we won't have boards with 2+ radio devices of the exact same kind (means using the same radio driver functions), which will be the case for most of them. I don't know if we want to restrict ourselves to this.

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Jul 28, 2014

Ah yes, now I see your problem. I totally forgot about that :/

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Jul 29, 2014

@thomaseichinger see #1492

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Aug 14, 2014

Rebased to current status of #1492.

@miri64 miri64 changed the title sys: Initial import of a general interface to a network protocol sys: net: Initial import of a general interface to a network protocol Aug 15, 2014
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Aug 19, 2014

Rebased and cleaned up PR a bit.

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Aug 19, 2014

Note: I'm aware that Travis is failing. Will fix that in a few hours ;-)

@miri64 miri64 force-pushed the network-layer-interface branch 4 times, most recently from fa256fc to ecd3610 Compare August 26, 2014 08:52
@miri64 miri64 force-pushed the network-layer-interface branch 2 times, most recently from 73d6fa0 to 4a7a728 Compare August 27, 2014 07:26
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Aug 27, 2014

Rebased due to b799292 and adapted dummy_mac for ISR event handling

@miri64 miri64 added State: waiting for other PR State: The PR requires another PR to be merged first and removed State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet labels Sep 8, 2014
@miri64 miri64 force-pushed the network-layer-interface branch from 2120d7f to f66b26d Compare September 9, 2014 23:07
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Use PRIkernel_pid instead of %d.

@OlegHahm
Copy link
Copy Markdown
Member

Be consistent with full-stops in the documentation.

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Nov 18, 2014

Addressed @OlegHahm's comments and added new functionality to the REG command.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

full-stop missing ;)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oops 😊

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Nov 19, 2014

Fixed last comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Isn't this member superfluous if it has to be NETAPI_CMD_RCV anyway?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No, otherwise you wouldn't be able to identify the incoming netapi_cmd_t as a netapi_rcv_pkt_t.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why not doing it with pointers like

typedef struct {
    netapi_cmd_type_t type;
    void *data;
} netapi_msg_t;

?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Why have another pointer (and 2-4 byte more for every command depending on the platform) deeper down when you can have all the data on the same level?

Other answer: because I use it like this (very successfully, see this file in #1680 e.g.) for months now and it would cost way more time than I'd like to put into it to change it to your approach ;-)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hm, I see.

@OlegHahm
Copy link
Copy Markdown
Member

Kicked Travis. Apart from the name of netapi_send_data2(): ACK.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@OlegHahm Use-case: Send data as it is intended to be: Payload and Headers seperated, a lower layer is in charge of the reassembly.

@miri64 miri64 force-pushed the network-layer-interface branch from ef93fa4 to 8ceb24e Compare November 20, 2014 13:31
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Nov 20, 2014

Rebased to master

@OlegHahm OlegHahm added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Nov 20, 2014
@OlegHahm
Copy link
Copy Markdown
Member

ACK. From my point of view, ready to be squashed and merged. @haukepetersen, you want to comment on this?

@miri64 miri64 force-pushed the network-layer-interface branch from 8ceb24e to 598e73b Compare November 20, 2014 13:36
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Nov 20, 2014

Squashed

@haukepetersen
Copy link
Copy Markdown
Contributor

no, just looked through it again, looks fine to me -> ACK

@miri64 miri64 removed the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Nov 20, 2014
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Nov 20, 2014

Very core-like for something that is not used by now :D

@miri64 miri64 force-pushed the network-layer-interface branch from 598e73b to b7a0794 Compare November 20, 2014 13:51
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Nov 20, 2014

Changed commit author.

miri64 added a commit that referenced this pull request Nov 20, 2014
sys: net: Initial import of a general interface to a network protocol
@miri64 miri64 merged commit 4c0aaa6 into RIOT-OS:master Nov 20, 2014
@miri64 miri64 deleted the network-layer-interface branch November 20, 2014 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: network Area: Networking Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants