Hello.
Thanks for your patch. Find my comments below.
On 14/11/16 04:09, Xue, Wenqian wrote:
Hi, all
Below shows my patch for the support to set wpan-ping interval, you can use the command `-I $interval(ms)` to set the interval value (default is 500ms).
If you find some errors or unexpected affects on current kernel, please feel free to point it out and give me some advice, thank you all!
Please use git send-email to send patches. That way I can properly
review and more important apply and test them here.
Bonus points if you use a wpan-tools prefix in the patch like this:
git format-patch --subject-prefix="PATCH wpan-tools" ...
diff --git a/wpan-ping.c b/wpan-ping.c
index 2c63758..4a0f00d 100644
--- a/wpan-ping.c
+++ b/wpan-ping.c
@@ -49,6 +49,7 @@
#define IEEE802154_ADDR_LEN 8
/* Set the dispatch header to not 6lowpan for compat */
#define NOT_A_6LOWPAN_FRAME 0x00
+#define DEFAULT_INTERVAL 500
#define DEBUG 0
@@ -96,6 +97,7 @@ struct config {
int nl802154_id;
struct sockaddr_ieee802154 src;
struct sockaddr_ieee802154 dst;
+ unsigned short interval;
};
extern char *optarg;
@@ -109,6 +111,7 @@ static void usage(const char *name) {
"--count | -c number of packets\n"
"--size | -s packet length\n"
"--interface | -i listen on this interface (default wpan0)\n"
+ "--interval | -I wait interval milliseconds between sending each packet (default 500ms)\n"
Maybe write: wait interval in milliseconds between sending packets
(default 500ms)
"--version | -v print out version\n"
"--help | -h this usage text\n", name);
}
@@ -232,6 +235,27 @@ static int print_address(char *addr, uint8_t dst_extended[IEEE802154_ADDR_LEN])
return 0;
}
+static void sleeping(struct timeval ping_start_time, struct timeval timeout)
+{
+ struct timeval curr_time;
+ long sec, usec, interval_usec, timeout_usec;
+ long sleep_usec = 0;
+ gettimeofday(&curr_time, NULL);
+ sec = curr_time.tv_sec - ping_start_time.tv_sec;
+ usec = curr_time.tv_usec - ping_start_time.tv_usec;
+ if (usec < 0)
+ {
+ usec += 1000000;
+ sec--;
+ }
+ interval_usec = sec * 1000000 + usec;
+ timeout_usec = timeout.tv_sec * 1000000 + timeout.tv_usec;
+ if (interval_usec < timeout_usec){
+ sleep_usec = timeout_usec - interval_usec;
+ usleep(sleep_usec);
+ }
+}
+
static int measure_roundtrip(struct config *conf, int sd) {
unsigned char *buf;
struct timeval start_time, end_time, timeout;
@@ -256,9 +280,17 @@ static int measure_roundtrip(struct config *conf, int sd) {
conf->dst.addr.short_addr, conf->dst.addr.pan_id, conf->packet_len);
buf = (unsigned char *)malloc(MAX_PAYLOAD_LEN);
- /* 500ms seconds packet receive timeout */
- timeout.tv_sec = 0;
- timeout.tv_usec = 500000;
+ /* default 500ms seconds packet receive timeout */
+ // timeout.tv_sec = 0;
+ // timeout.tv_usec = 500000;
Please do not leave commented out code here. You moved the default value
into a define and use it accordingly so you can safely remove this here.
+ if (conf->interval >= 1000){ // when interval is larger than 1s
No C++ style comments please. Use /* */
+ double intervalD = (conf->interval * 1.0) / 1000;
intervalD as variable name is not very nice. Please choose something else.
+ timeout.tv_sec = (int)intervalD;
+ timeout.tv_usec = (int)((intervalD - timeout.tv_sec) * 1000000);
+ } else {
+ timeout.tv_sec = 0;
+ timeout.tv_usec = conf->interval * 1000;
+ }
ret = setsockopt(sd, SOL_SOCKET, SO_RCVTIMEO, (struct timeval *)&timeout,sizeof(struct timeval));
if (ret < 0) {
perror("setsockopt receive timeout");
@@ -266,6 +298,7 @@ static int measure_roundtrip(struct config *conf, int sd) {
count = 0;
for (i = 0; i < conf->packets; i++) {
+ gettimeofday(&ping_start_time, NULL);
generate_packet(buf, conf, i);
seq_num = (buf[2] << 8)| buf[3];
ret = sendto(sd, buf, conf->packet_len, 0, (struct sockaddr *)&conf->dst, sizeof(conf->dst));
@@ -309,7 +342,9 @@ static int measure_roundtrip(struct config *conf, int sd) {
fprintf(stdout, "%i bytes from 0x%04x seq=%i time=%.1f ms\n", ret,
conf->dst.addr.short_addr, (int)seq_num, (float)usec/1000);
} else
- fprintf(stderr, "Hit 500 ms packet timeout\n");
+ fprintf(stderr, "Hit %d ms packet timeout\n", conf->interval);
+ // sleeping
C++ comment. See above.
+ sleeping(ping_start_time, timeout);
}
if (count)
@@ -456,6 +491,9 @@ int main(int argc, char *argv[]) {
/* Default to 65535 packets being sent */
conf->packets = USHRT_MAX;
+ /* Default to 500ms for interval value */
+ conf->interval = DEFAULT_INTERVAL;
+
if (argc < 2) {
usage(argv[0]);
exit(1);
@@ -464,9 +502,9 @@ int main(int argc, char *argv[]) {
while (1) {
#ifdef _GNU_SOURCE
int opt_idx = -1;
- c = getopt_long(argc, argv, "a:ec:s:i:dvh", perf_long_opts, &opt_idx);
+ c = getopt_long(argc, argv, "a:ec:s:i:dvhI:", perf_long_opts, &opt_idx);
#else
- c = getopt(argc, argv, "a:ec:s:i:dvh");
+ c = getopt(argc, argv, "a:ec:s:i:dvhI:");
#endif
if (c == -1)
break;
@@ -495,6 +533,9 @@ int main(int argc, char *argv[]) {
case 'i':
conf->interface = optarg;
break;
+ case 'I':
+ conf->interval = atoi(optarg);
+ break;
case 'v':
fprintf(stdout, "wpan-ping " PACKAGE_VERSION "\n");
free(conf);
Please change what I mentioned above and resend this patch with git
send-email. Write a good commit message and do not forgot the signed-off
line (-s for git format-patch can also generate this for you)
The change itself looks ok on a first glance at the code but I need to
actual compile and test it.
regards
Stefan Schmidt
--
To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
