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

Shawn Radio: Unregister message callback #8

Open
gitbaum opened this issue Oct 12, 2011 · 17 comments
Open

Shawn Radio: Unregister message callback #8

gitbaum opened this issue Oct 12, 2011 · 17 comments

Comments

@gitbaum
Copy link
Contributor

gitbaum commented Oct 12, 2011

Implement the method unreg_recv_callback, currently it is:

int unreg_recv_callback( int idx )
{ return ERR_NOTIMPL; }

@shubhamsomani
Copy link
Contributor

I was checking this issue and I'm stuck on it as I am unable to find the function which would unregister the receiver.
When I go to http://www.ibr.cs.tu-bs.de/trac/wiselib/browser/trunk/shawn_apps/wiselib/ext_iface_processor.h?rev=14912 I see that there is no such function. Please Help.

@konstantinosch
Copy link
Contributor

Hi

There is no receiver unregistration from what I know. You could limit
the handling of your "receive" like function with a flag that is
activated in the same place you would theoretically call ur unregister
function. In order to include such a thing you have to do some
re-design in the way of the registration. Like having a struct with a
unique ID and the function-callback pointer and some generic settings
maybe.

you can check the protocol class that works like that

https://github.com/ibr-alg/wiselib/blob/master/wiselib.testing/algorithms/neighbor_discovery/protocol.h

and the register/unregister functions in here

https://github.com/ibr-alg/wiselib/blob/master/wiselib.testing/algorithms/neighbor_discovery/neighbor_discovery.h

Kostas

On 12 February 2013 12:11, Shubham Somani [email protected] wrote:

I was checking this issue and I'm stuck on it as I am unable to find the function which would unregister the receiver.
When I go to http://www.ibr.cs.tu-bs.de/trac/wiselib/browser/trunk/shawn_apps/wiselib/ext_iface_processor.h?rev=14912 I see that there is no such function. Please Help.


Reply to this email directly or view it on GitHub.

@konstantinosch
Copy link
Contributor

Looks good, but

  1. try to keep the old interface intact along with this.

  2. I could complain about the use of array instead of a pstl vector but
    there is no dynamicity in wiselib with memory to have any sense. On the
    other hand on unregistration you are stuck with the proc->id() entry inside
    your array. Using a pstl vector u can just delete that with erase() and
    look cool as well :)

Kostas

On 14 February 2013 20:16, Shubham Somani [email protected] wrote:

Thanks Kostas.
Tried implementing it , but since I was unsure of my implementation I
thought of getting it reviewed first before sending a pull request. Please
See http://pastebin.com/XtnxfV63.


Reply to this email directly or view it on GitHubhttps://github.com//issues/8#issuecomment-13572985.

@shubhamsomani
Copy link
Contributor

Thanks Kostas.
Tried implementing it , but since I was unsure of my implementation I
thought of getting it reviewed first before sending a pull request. Please
See http://pastebin.com/XtnxfV63.

@shubhamsomani
Copy link
Contributor

Thanks :)

@konstantinosch
Copy link
Contributor

also watch out this node_id indexing you do. I use 2byte random id's in my
nodes and not a sequence. that would require a big array for no reason.

unless you do it with a nice pstl vector and u just push back...

On 14 February 2013 20:33, Shubham Somani [email protected] wrote:

Thanks :)


Reply to this email directly or view it on GitHubhttps://github.com//issues/8#issuecomment-13573867.

@shubhamsomani
Copy link
Contributor

I get your point. I'll change the array to a pstl vector...

@shubhamsomani
Copy link
Contributor

@kostas
A little help needed....
Does a pstl vector support struct as type? (or am i doing something wrong? vector_static< OsModel,s,40 > a;)

@konstantinosch
Copy link
Contributor

Yes you can (any datatype actually) also see at the examples I gave you in
neighbor_discovery.h and protocol.h
I did it like that but instead of struct I used a class. It's the same
concept.

Kostas

On 14 February 2013 21:15, Shubham Somani [email protected] wrote:

@kostas https://github.com/kostas
A little help needed....
Does a pstl vector support struct as type? (or am i doing something wrong?
vector_static a;)


Reply to this email directly or view it on GitHubhttps://github.com//issues/8#issuecomment-13575986.

@shubhamsomani
Copy link
Contributor

Revised code with pstl vectors. Please point out any errors left. http://pastebin.com/qMiTs8LQ. Thanks.

@konstantinosch
Copy link
Contributor

seems correct but you shouldn't use the node id as an id for a registered
callback.

If you have a protocol A that registers for callbacks and a protocol B that
registers for callbacks to the same Radio interface, which one are you
going to be erasing? They will both have the same id derived from the node
id.
since you want unique id's, your protocols should have unique id's to
define them. dont use the size_t vector.size() out of intuition and don't
bother with random id's (they could produce non-unique values).
for example

    enum protocol_id's
    {
               JOHNS_ROUTING,
               MARY_FLOODING,
               MICHAEL_GROUP_KEY,
               HELEN_CLUSTERING
    };

Also its better to make typedef struct s, so that the Protocol that
registers for callbacks can instantiate the s datatype and the pass it as a
parameter to the register / unregister function.
For example your protocol is picking the Radio interface then it grabs the
typedefed type.

something like that would be cleaner i guess.

typedef typename Radio::s s;
s my_callback_struct;

radio().register(s)
radio().unregister(s)

flag is useless unless you want to finetune enable/disable functionality
internally - but you can always unregister/register.

oh and

reciever should be receiver :P

Kostas

ps. good job

On 14 February 2013 22:34, Shubham Somani [email protected] wrote:

Revised code with pstl vectors. Please point out any errors left.
http://pastebin.com/qMiTs8LQ. Thanks.


Reply to this email directly or view it on GitHubhttps://github.com//issues/8#issuecomment-13579930.

@shubhamsomani
Copy link
Contributor

Thanks Kostas for seeing the revision and giving a better viewpoint.

Umm (from what i understand) you want me to change the struct to typedefed type so that it can be instantiated and I should change my register and unregister functions so that they accept this struct as a parameter. right?

What I don't understand is since node ID's can be duplicated...what should i use to compare with the vector list while unregistering?

@konstantinosch
Copy link
Contributor

Hi again,

On 16 February 2013 11:59, Shubham Somani [email protected] wrote:

Thanks Kostas for seeing the revision and giving a better viewpoint.

Umm (from what i understand) you want me to change the struct to typedefed
type so that it can be instantiated and I should change my register and
unregister functions so that they accept this struct as a parameter. right?

exactly

What I don't understand is since node ID's can be duplicated...what should
i use to compare with the vector list while unregistering?

As I said in the previous message it's better to assign an ID explicitly
for each protocol that is instantiated in a node and registers for
callbacks in a Radio or any other protocol that supports callbacks.
*
*
Remeber that a node is not registering for callbacks. An arbitrary amount
of instantiated protocols inside the node firmware could register for
callbacks.

This is also how shawn works similarly to other wsn nodes people use in
experimental testbeds.

Now the struct s parameter could be a reference to keep things more light.

  1.   int unreg_recv_callback( struct s& my_struct )
    
  2.   {
    
  3.       recievers.erase(recievers.ID.find(my_struct.id));
    
  4.       return SUCCESS;
    
  5.   }
    

something like this for instance


Reply to this email directly or view it on GitHubhttps://github.com//issues/8#issuecomment-13648070.

@shubhamsomani
Copy link
Contributor

Thanks for helping again.
http://pastebin.com/E0ZeixpE
Its pretty similar to the last one .

  1. Changed the way ID's are being registered.
  2. I have my reservations on whether reg_recv_callback should receive struct as a parameter because it would lead to changing the function call in all the files that were calling it earlier. (Please tell me if the 2nd point is invalid, in that case I would change my function accordingly.)

@konstantinosch
Copy link
Contributor

Hi, i took a step back to understand your general rationale.

  1. you want to include unregister functionality to wiselib?
  2. you want to include unregister functionality to this specific radio in
    shawn?
  3. you want to make your own radio that supports unregister functionality?

Cause all my answers so far are more focused to a solution for (3). if it's
about (2) then you have to maintain its interface in regards to register
unregister methods to support any depended code and include what we talked
about so far as EXTRA functionality. overload is the keyword.

On 16 February 2013 20:03, Shubham Somani [email protected] wrote:

Thanks for helping again.
http://pastebin.com/E0ZeixpE
Its pretty similar to the last one .

  1. Changed the way ID's are being registered.

just keep it simple with the id's. with the incrementing you do now, what
happens when you unregister id 5000 in your 10000 element container? you
update all the id's? Just keep it simple. Don't try to automate id
assignment and dont try to control duplicates. Let the people who register
in the radio worry for that by explicitly setting their
derived-from-radio-typed-struct-member id.

  1. I have my reservations on whether reg_recv_callback should receive
    struct as a parameter because it would lead to changing the function call
    in all the files that were calling it earlier. (Please tell me if the 2nd
    point is invalid, in that case I would change my function accordingly.)

well the way it is now how do you unregister with an id that is set
internally and out of sight ...?


Reply to this email directly or view it on GitHubhttps://github.com//issues/8#issuecomment-13667164.

Kostas

@shubhamsomani
Copy link
Contributor

Thanks Kostas for replying again. You have really been very helpful.

I think i've implemented (3) correctly http://pastebin.com/jrR8Wn9h

  1. I thought I was implementing (2) but now I get that it was my misconception. My bad.

  2. My idea with the ID increment thing was that it would be kind of a hash where values which have been assigned would me marked. If suppose 5000 is unregistered then that value would be deleted.Why would I need to update the whole vector for that? Only the values which are CURRENTLY registered would be in the vector. And when this assignment would reach maximum limit ,it would again start from zero, like a cycle. But you correctly pointed out a problem of how the user will able to unregister with an ID out of his sight. Would it be possible to return the ID that we mark internally so that he can use it while unregistering? (or this idea is just a bad one)

  3. changed reciever should be receiver :P

@shubhamsomani
Copy link
Contributor

the 2nd point of my comment is regarding unregister functionality to this specific radio in
shawn

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants