-
Notifications
You must be signed in to change notification settings - Fork 8.1k
modem: ppp: use peer control character map #97963
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
base: main
Are you sure you want to change the base?
Conversation
5683788
to
4cc4705
Compare
subsys/net/l2/ppp/ppp_l2.c
Outdated
struct ppp_context *ctx = net_if_l2_data(iface); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As this is a public facing API, we could add extra checks to make sure that only PPP interface is being worked on
struct ppp_context *ctx;
if (net_if_l2(iface) != &NET_L2_GET_NAME(PPP)) {
return 0xffffffff;
}
ctx = net_if_l2_data(iface);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added an assertion in the same style. I can't see much use for this outside of the PPP drivers themselves, so there really shouldn't be anyone calling this function not on a PPP L2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, agreed. It is just that the prototype is placed in public header file and the API documentation is generated for it. We could avoid that by placing the function declaration to inside @cond ... @endcond
block so that the documentation is not generated for it.
Anyway, that is a minor detail.
Expose the currently configured value for the PPP peers Asynchronous Control Character Map through a public function. Signed-off-by: Jordan Yates <[email protected]>
Accept the `ASYNC_CTRL_CHAR_MAP` option in configuration messages from the peer. The map is reset to the default value each time the interface comes up. Signed-off-by: Jordan Yates <[email protected]>
Use the peers asychronous control character map to only escape characters that need to be, instead of always escaping bytes 0 to 31. For data packets with values randomly distributed, this results in 11% fewer bytes needing to be transmitted over the link, if no bytes need to be escaped (256+2)/(256+2+32). Signed-off-by: Jordan Yates <[email protected]>
4cc4705
to
6d895c5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but need to figure out how to fix the test, it seems that the assert broke them. Not sure how to tackle this best, perhaps assert only if CONFIG_NET_TEST
is disabled?CONFIG_NET_TEST
would need to be added to the prj.conf
in the test though.
Test the wrapping functions with custom asynchronous command character maps. Signed-off-by: Jordan Yates <[email protected]>
6d895c5
to
05e90d3
Compare
Wrapped the assert in |
|
Use the peers asynchronous control character map to only escape characters that need to be, instead of always escaping bytes 0 to 31.
For data packets with values randomly distributed, this results in 11% fewer bytes needing to be transmitted over the link, if no bytes need to be escaped (256+2)/(256+2+32).
Validated on a Telit LE910C1, which attempts to configure the control character map as
0x00000000
.