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

tools/zep_dispatch: enhancements and fixes #19996

Merged
merged 12 commits into from
Jan 4, 2024

Conversation

benpicco
Copy link
Contributor

Contribution description

This is a collection of improvements I added while working on the CoAP multicast implementation.

topogen

  • binary mode: links will be dropped below a cutoff-point, above it they are always successful
  • numeric node names: allows for easier scripting
  • make link quality drop off quadratically with distance
  • always place the first node at a known location

dispatcher

  • fix bug where packet won't get delivered to any node after it was dropped on a single link
  • count rx/tx frames per node
  • add possibility to store pidfile

Testing procedure

Issues/PRs references

@github-actions github-actions bot added the Area: tools Area: Supplementary tools label Oct 19, 2023
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Nov 1, 2023
@benpicco benpicco requested review from miri64 and kfessel November 1, 2023 13:11
@riot-ci
Copy link

riot-ci commented Nov 1, 2023

Murdock results

✔️ PASSED

ba8130e tools/zep_dispatch: topogen: add help text

Success Failures Total Runtime
1 0 1 01m:04s

Artifacts

@kfessel
Copy link
Contributor

kfessel commented Nov 1, 2023

could you sketch a Testing procedure

Copy link
Contributor

@kfessel kfessel left a comment

Choose a reason for hiding this comment

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

just some comments

dist/tools/zep_dispatch/topogen.c Outdated Show resolved Hide resolved
dist/tools/zep_dispatch/topogen.c Outdated Show resolved Hide resolved
@@ -13,6 +13,10 @@
#include <math.h>
#include <unistd.h>

#ifndef CONFIG_USE_NUMERIC_NAMES
#define CONFIG_USE_NUMERIC_NAMES 1
Copy link
Contributor

Choose a reason for hiding this comment

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

why isn't that a comandline option

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, I wasn't sure if I shouldn't remove it altogether.
The alphabetic names look nice for node numbers < 26, but once we use two letters, it's much more confusing than just numbering them.

Copy link
Contributor

Choose a reason for hiding this comment

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

Lets keep them in hidden

@@ -51,7 +72,7 @@ static double node_distance(const struct node *a, const struct node *b)

static double node_distance_weight(const struct node *a, const struct node *b)
{
double w = 1 - node_distance(a, b) / a->r;
double w = 1 - pow(node_distance(a, b), 2) / pow(a->r, 2);
Copy link
Contributor

Choose a reason for hiding this comment

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

will this surprise people? (changing distance from linear to quadratic (more realistic))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tbh I don't think there is currently anyone besides me using this

Copy link
Contributor

@kfessel kfessel left a comment

Choose a reason for hiding this comment

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

I think benpicco tested this well.
Since benpicco is the main author of this application and I can't see special/strange/surprising changes just enhancements and a slight change in distance calculation.

please squash and merge these.

@OlegHahm
Copy link
Member

I would be interested at some point to discuss whether this tooling and des-virt can be merged somehow. It's a bit weird that we have (at least) two different network emulators in the repository - and apparently both them are used by only very few people.

@benpicco benpicco added this pull request to the merge queue Jan 3, 2024
@benpicco benpicco removed this pull request from the merge queue due to a manual request Jan 3, 2024
@benpicco benpicco added this pull request to the merge queue Jan 3, 2024
Merged via the queue into RIOT-OS:master with commit b47771c Jan 4, 2024
25 checks passed
@benpicco benpicco deleted the topogen_enhance branch January 4, 2024 11:53
@MrKevinWeiss MrKevinWeiss added this to the Release 2024.01 milestone Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: tools Area: Supplementary tools CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants