Skip to content

Conversation

@jukkar
Copy link
Member

@jukkar jukkar commented Feb 13, 2019

This allows application(s) to listen same CAN-IDs for the same network interface.

@jukkar jukkar added this to the v1.14.0 milestone Feb 14, 2019
@pfalcon
Copy link
Contributor

pfalcon commented Feb 25, 2019

This needs rebase.

@jukkar jukkar force-pushed the socket_can_dispatcher branch from c8a056b to 175b459 Compare February 27, 2019 15:18
@codecov-io
Copy link

codecov-io commented Feb 27, 2019

Codecov Report

Merging #13375 into master will decrease coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #13375      +/-   ##
==========================================
- Coverage   52.02%   51.99%   -0.04%     
==========================================
  Files         309      309              
  Lines       45574    45574              
  Branches    10555    10555              
==========================================
- Hits        23711    23695      -16     
- Misses      17057    17069      +12     
- Partials     4806     4810       +4
Impacted Files Coverage Δ
subsys/net/ip/net_context.c 49.41% <ø> (ø) ⬆️
subsys/net/ip/connection.c 71.86% <ø> (ø) ⬆️
subsys/testsuite/ztest/include/ztest_assert.h 38.88% <0%> (-38.89%) ⬇️
subsys/testsuite/ztest/src/ztest.c 65.28% <0%> (-7.44%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a1c2159...7dc52cb. Read the comment docs.

@jukkar jukkar force-pushed the socket_can_dispatcher branch from 175b459 to b3ad6ed Compare March 11, 2019 17:06
@jukkar
Copy link
Member Author

jukkar commented Mar 11, 2019

@jukkar jukkar force-pushed the socket_can_dispatcher branch from b3ad6ed to 7dc52cb Compare March 15, 2019 08:48
@jukkar
Copy link
Member Author

jukkar commented Mar 15, 2019

@alexanderwachter can you have a re-review?

Copy link
Member

@alexanderwachter alexanderwachter Mar 15, 2019

Choose a reason for hiding this comment

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

Don't you need to check for i != 0 instead of ARRAY_SIZE(receivers) > 1 here?
Because the original pkt must be passed so it is unrfed somewhere.

Copy link
Member Author

Choose a reason for hiding this comment

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

Don't you need to check for i != 0 instead of ARRAY_SIZE(receivers) > 1 here?

In order to simplify pkt housekeeping, I just cloned the packet if there are multiple receiver. After revisiting this one after some time, it looks quite non-optimal. I will rework this a bit more.

Copy link
Member

Choose a reason for hiding this comment

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

Still I would keep the original pkt and only clone if i > 0.
But this works too.

@alexanderwachter
Copy link
Member

When you send a message you should check the registered filters and if the id matches, the message should be cloned to this receiver.
What about detaching the filters after the socket is closed? I can't find this in the actual implementation.

@jukkar jukkar modified the milestones: v1.14.0, v1.15.0 Mar 15, 2019
@jukkar
Copy link
Member Author

jukkar commented Mar 15, 2019

Lets target this for 1.15

@jukkar jukkar force-pushed the socket_can_dispatcher branch from 7dc52cb to a86f08a Compare April 17, 2019 20:38
@jukkar
Copy link
Member Author

jukkar commented Apr 17, 2019

Rebased against latest master

@jukkar
Copy link
Member Author

jukkar commented Apr 23, 2019

This still needs some TLC as @alexanderwachter comments needs to be addressed. I am working on it but the progress is slow because of other pending tasks.

@jukkar jukkar force-pushed the socket_can_dispatcher branch from a86f08a to a5c27d8 Compare May 16, 2019 19:03
@jukkar
Copy link
Member Author

jukkar commented May 16, 2019

Fixed merge conflicts, no other changes yet.

@jukkar jukkar force-pushed the socket_can_dispatcher branch from a5c27d8 to c4a0160 Compare May 20, 2019 08:33
@jukkar
Copy link
Member Author

jukkar commented May 20, 2019

Updated according to comments.

@jukkar jukkar force-pushed the socket_can_dispatcher branch from c4a0160 to 227f889 Compare May 20, 2019 09:41
@jukkar
Copy link
Member Author

jukkar commented May 20, 2019

Updated according to comments.

@jukkar jukkar force-pushed the socket_can_dispatcher branch from 227f889 to fb8ba8f Compare June 9, 2019 08:05
@zephyrbot zephyrbot added the area: Samples Samples label Jun 9, 2019
@jukkar
Copy link
Member Author

jukkar commented Jun 9, 2019

@alexanderwachter can you review this, all the comments you had should be ok now.

@alexanderwachter
Copy link
Member

There is still no filter detaching when the socket is closed.
The filter is still attached if an application for example opens a socket, set the filter socket opt and then closes the socket. If the application does this several times we would run out of filters very quickly. Another problem with leaving the filter attaches is that this filter causes interrupts even nobody is waiting for this frame.

jukkar added 7 commits June 18, 2019 15:40
We need to dispatch the received CAN frame if there are multiple
sockets interested in the same CAN-IDs.

Signed-off-by: Jukka Rissanen <[email protected]>
This is done so that we can test socket CAN dispatcher.

Signed-off-by: Jukka Rissanen <[email protected]>
At the moment there is no real address for local CANBUS socket,
but we can still set protocol family of local socket to AF_CAN
so that for example net-shell "net conn" command can show
information about it.

Signed-off-by: Jukka Rissanen <[email protected]>
CANBUS socket information was just printing unknown information.

Signed-off-by: Jukka Rissanen <[email protected]>
We need to initialize the connection.c for UDP, TCP, PACKET socket
and CANBUS sockets.

Signed-off-by: Jukka Rissanen <[email protected]>
If the socket is closed, then do CAN detach if that is needed.
This way the CAN interrupts are not received if there are no
CAN sockets listening the data.

Signed-off-by: Jukka Rissanen <[email protected]>
This is done only for testing purposes, in real life the socket
would be closed if it is not used or needed.

Signed-off-by: Jukka Rissanen <[email protected]>
@jukkar jukkar force-pushed the socket_can_dispatcher branch from fb8ba8f to 8bf148b Compare June 18, 2019 12:44
@jukkar
Copy link
Member Author

jukkar commented Jun 18, 2019

Fixed the socket closing (detach filters if needed). See "net: sockets: can: Close the socket cleanly" for the implementation. The sample app was changed to do periodic closes in order to test this.

@zephyrbot zephyrbot added the area: API Changes to public APIs label Jun 18, 2019
@jukkar jukkar merged commit 404ac51 into zephyrproject-rtos:master Jun 18, 2019
@jukkar jukkar deleted the socket_can_dispatcher branch June 18, 2019 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants