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

sys/shell: added UDP shell commands #2720

Closed
wants to merge 2 commits into from

Conversation

haukepetersen
Copy link
Contributor

based on #2719

This code is not compilable/runnable at the moment, as it depends on some IPv6 and UDP functionality, which is not yet merged...

Basically this code serves two purposes: (i) to have simple to use shell commands for sending and receiving UDP packets and (ii) for having a simple example on howto do this :-)

Update:
depends on #2430

@haukepetersen haukepetersen added Type: new feature The issue requests / The PR implemements a new feature for RIOT NSTF labels Mar 26, 2015
return;
}
/* allocate IPv6 header */
ip = ng_ipv6_hdr_build(udp, NULL, 0, (uint8_t *)&ip_addr, sizeof(ip_addr));
Copy link
Member

Choose a reason for hiding this comment

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

Would prefer use of ng_netreg_hdr_build() here, so we later can swap the IP header type easily depending on the format of the IP address.

Copy link
Member

Choose a reason for hiding this comment

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

(also: wouldn't require #2454 for this to compile, but only #2575)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

as long as we do not have support for IPv4, I do not really see the necessity of introducing an additional function call. Once IPv4 is introduced, some of this code has to be re-factored anyhow...

@haukepetersen
Copy link
Contributor Author

made use of ng_ipv6_addr_from_str()

@@ -159,6 +159,10 @@ extern void _netif_send(int argc, char **argv);
#endif
#endif

#ifdef MODULE_NG_UDP
extern void _udp(int argc, char **argv);
Copy link
Member

Choose a reason for hiding this comment

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

Just a heads-up: shell-handlers have the signature

int void <name>(int argc, char **argv);

now ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yap, I saw that (though I have no idea why that was introduced...).

@OlegHahm OlegHahm force-pushed the master branch 3 times, most recently from 9f184dd to 45554bf Compare March 31, 2015 13:01
@OlegHahm
Copy link
Member

As far as I can see this PR contains more than just adding the shell command, or am I mistaken?

For the shell command itself, I think this is typical case for #2755.

@OlegHahm OlegHahm added the State: waiting for other PR State: The PR requires another PR to be merged first label Apr 21, 2015
@miri64
Copy link
Member

miri64 commented Apr 21, 2015

Yes #2719 is still in here (which was merged). @haukepetersen please rebase to master.

@miri64 miri64 removed the State: waiting for other PR State: The PR requires another PR to be merged first label Apr 21, 2015
@haukepetersen
Copy link
Contributor Author

will do.

@OlegHahm OlegHahm added the State: waiting for other PR State: The PR requires another PR to be merged first label Apr 24, 2015
@haukepetersen
Copy link
Contributor Author

rebased. It still depends on #2430.

@miri64
Copy link
Member

miri64 commented Apr 26, 2015

Come to think about it, why do are these commands automatically compiled in? They are more or less just test commands. Why aren't they just part of e.g. a UDP test application.

@miri64
Copy link
Member

miri64 commented Apr 26, 2015

(if someone wants to use them in their application they can just copy it from there)

@haukepetersen
Copy link
Contributor Author

in the end, everything related to the shell is more or less for testing... How about I change this PR into a UDP example application?

@miri64
Copy link
Member

miri64 commented Apr 26, 2015

Yes and no. Most are for configuration or bootstrapping. I'd prefer the example application.

@OlegHahm
Copy link
Member

btw that's what I meant with

For the shell command itself, I think this is typical case for #2755.

@haukepetersen
Copy link
Contributor Author

Ok. The only problem at the moment is, that we do not have a network stack bootstrapping/auto_init mechanism in place. So a sample application will look most ugly at the moment... I will spend some time on the init stuff today and see if I can finally come up with something.

@miri64
Copy link
Member

miri64 commented Apr 27, 2015

The only thing still missing is device and MAC layer initialization. Everything above bootstraps already via their respective init functions called from auto_init

@haukepetersen
Copy link
Contributor Author

Yap, but thats the interesting part :-) I will propose something here soon.


void _send(char *addr, char *port, char *data)
{
uint16_t port;
Copy link
Member

Choose a reason for hiding this comment

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

Is that normal that port is redefined here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

probably not (This PR is not tested too well, and as we discussed above will be heavily re-structured anyway...)

@haukepetersen
Copy link
Contributor Author

I close this in favor of #2895.

@haukepetersen haukepetersen deleted the ng_scudp branch April 30, 2015 15:25
@miri64 miri64 added the Area: network Area: Networking label Sep 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: network Area: Networking State: waiting for other PR State: The PR requires another PR to be merged first 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.

4 participants