tasklwIP - A Lightweight TCP/IP stack - Tasks: task #14283, Add more detailed status callback

 
 

You are not allowed to post comments on this tracker with your current authentication level.

task #14283: Add more detailed status callback

Submitter:  Simon Goldschmidt <goldsimon>
Submitted:  Thu 22 Dec 2016 09:01:45 AM UTC
   
 
Category:  None Should Start On:  Thu 22 Dec 2016 12:00:00 AM UTC
Should be Finished on:  Thu 22 Dec 2016 12:00:00 AM UTC Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  dziegel Percent Complete:  100%
Open/Closed:  Closed Planned Release:  None
Effort:  0.00

Jump to the original submission

Mon 20 Feb 2017 06:27:41 PM UTC, comment #7: 

No further comments -> done

Dirk Ziegelmeier <dziegel>
Group administrator
Thu 16 Feb 2017 07:16:17 AM UTC, comment #6: 

The union is a good idea. However, it makes calling the callback function a but ugly since one needs to create an instance of the union on the stack, fill it and pass a pointer to the callback. But all-in-all, I guess it's an improvement.

Once again now for the improved version:
Comments/wishes/improvements anyone? (Look at bottom of netif.h)

Dirk Ziegelmeier <dziegel>
Group administrator
Tue 14 Feb 2017 10:45:14 PM UTC, comment #5: 

Dirk,

I took a look over the API and it looks like a great improvement over the other callbacks, especially providing the old IP address for some of the notifications.

My only feedback is I wonder if the num/arg can be improved.  If I understood correctly, num is really just a second arg that is not a pointer, so you have two possible arguments which the application has to interpret from the documentation

Would something like a union allocated on the stack and then passing a pointer to maybe be easier for applications to work with (also allow future growth in parameters)?  For callbacks that don't pass any data, they could send NULL.  I mocked this up:


union netif_nsc_reason_data {
  /* Valid for LWIP_NSC_LINK_CHANGED
               LWIP_NSC_STATUS_CHANGED */
  u8_t state; /* 1 for up, 0 for down */

  /* Valid for: LWIP_NSC_IPV4_ADDRESS_CHANGED
                LWIP_NSC_IPV4_GATEWAY_CHANGED
                LWIP_NSC_IPV4_NETMASK_CHANGED */
  ip_addr_t* old address;

  /* Valid for: LWIP_NSC_IPV6_SET
                LWIP_NSC_IPV6_ADDR_STATE_CHANGED */
  struct {
    u8_t index;
    ip_addr_t* old address;
  } ipv6;
}


Joel Cunningham <jcunningham>
Group Member
Tue 14 Feb 2017 08:15:19 PM UTC, comment #4: 

Restructured, function call has less arguments now. new_ip can be obtained from netif, so no need to supply it in call args.

Comments/wishes/improvements anyone? (Look at bottim of netif.h)

Dirk Ziegelmeier <dziegel>
Group administrator
Mon 13 Feb 2017 07:39:08 AM UTC, comment #3: 

Looks good.
- I think LWIP_NSC_NETIF_ADDED is missing
- IPv4 Address/mask/gw should be one callback (if set together)
- IPv6 address change should include an index (in the void*?)
- LWIP_NSC_NETIF_IPV6_ address status change is missing

Simon Goldschmidt <goldsimon>
Group administrator
Sun 12 Feb 2017 08:01:42 PM UTC, comment #2: 

- Is the function signature OK? Should we omit the void* arg? It makes the API more extensible.

- The callback is designed in a way that multiple clients can be registered. This should IMHO be supported.

- How do we handle events like DHCP address/gw/mask changed? Three callbacks or only one?

Dirk Ziegelmeier <dziegel>
Group administrator
Sun 12 Feb 2017 07:48:21 PM UTC, comment #1: 

API Proposal for discussion:


#if LWIP_NETIF_EXT_STATUS_CALLBACK
typedef enum
{
  /** netif was removed. arg is NULL */
  LWIP_NSC_NETIF_REMOVED,
  /** link changed. arg: 1 up, 0 down */
  LWIP_NSC_LINK_CHANGED,
  /** netif status changed. arg: 1 up, 0 down */
  LWIP_NSC_NETIF_STATUS_CHANGED,
  /** IPv4 address has changed. arg is NULL */
  LWIP_NSC_NETIF_IPV4_ADDRESS_CHANGED,
  /** IPv4 gateway has changed. arg is NULL */
  LWIP_NSC_NETIF_IPV4_GATEWAY_CHANGED,
  /** IPv4 netmask has changed. arg is NULL */
  LWIP_NSC_NETIF_IPV4_NETMASK_CHANGED,
  /** called AFTER IPv4 address/gateway/netmask changes have been applied */
  LWIP_NSC_NETIF_IPV4_SETTINGS_CHANGED,
  /** IPv6 address was added. arg is NULL */
  LWIP_NSC_NETIF_IPV6_ADDED,
  /** IPv6 address was removed. arg is NULL */
  LWIP_NSC_NETIF_IPV6_REMOVED
} netif_status_callback_reason_t;

typedef void (*netif_ext_status_callback)(struct netif* netif, lwip_netif_status_callback_reason_t reason, void* arg, const ip_addr_t* old, const ip_addr_t* new);

struct netif_ext_callback;
typedef struct netif_ext_callback
{
  netif_ext_status_callback callback;
  struct netif_ext_callback* next;
} netif_ext_callback_t;

void netif_add_ext_callback(netif_ext_callback_t* callback);
void netif_remove_ext_callback(netif_ext_callback_t* callback);
void netif_invoke_ext_callback(struct netif* netif, lwip_netif_status_callback_reason_t reason, void* arg, const ip_addr_t* old, const ip_addr_t* new);
#endif


Dirk Ziegelmeier <dziegel>
Group administrator
Thu 22 Dec 2016 09:01:45 AM UTC, original submission:  

- integrate link, up/down and IP added/removed status into one callback
- provide information about what's changed (IPv4/IPv6, which address index, old & new address on address change, etc)

- add listeners at runtime? (that could be additional)

Simon Goldschmidt <goldsimon>
Group administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jcunningham (Posted a comment)
  • -email is unavailable- added by dziegel (Posted a comment)
  • -email is unavailable- added by goldsimon (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-02-20 dziegel StatusReady For Test Done
        Open/ClosedOpen Closed
    2017-02-16 dziegel Percent Complete0% 100%
    2017-02-14 dziegel StatusIn Progress Ready For Test
    2017-02-13 dziegel StatusNone In Progress
        Assigned toNone dziegel

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code