update(): new implementation - frame type handling

[reixd]

Currently the update() function encapsulated the following activities:

• if available get frames from radio
• check frame
• check address
• frame type handling

I propose to split this function activities into functions to handle exclusively each action.
Additionally handling different frame types could be also splitted into different functions.

This measure could make the code easy to read and implementing different frame types would be easy: define new type, add type to switch-case, define new type handling function.

Would be this code changes a problem using arduino?

[TMRh20]

Well, I guess it depends on exactly how far you are looking to extend the current functionality, and what ideas you have for additional frame types. One of the biggest challenges in extending the RF24Network library is ensuring compatibility with devices like Arduino and ATTiny85, since they can be used as routing nodes etc. so trying to keep memory usage and code size down in that regard is very important.

I'm not entirely sure that completely breaking up the update() function would simplify things, since it is the starting point for almost every non-user initiated action, unless a large number of frame types is added etc.

One thing that is bugging me about the current implementation, is that using fragmentation currently breaks the feature of user-defined header types, since it replaces that field with the fragmentation types. For example, if I define a header of type T, and add a large payload, my user defined type is replaced with the system fragmentation types. It doesn't really affect the RF24toTUN implementation, but is an important feature otherwise. I think this should be addressed prior to extending the current functionality, but haven't looked at a solution yet.

[reixd]

I understand the problems with arduino, therefore my willing to start a discussion before taking action.

In regard to the flags issue, you are right: that behavior is a bug in my eyes. It broke previous design without warning current users.

A have two proposals:

• use bit masks for the header.type
  • easy to implement
  • but not really user friendly
  • reduces the number of possible flags drastically
  • API change means users need to update their software -> bad
• leave header.type alone and use header.fragment_id
  • use first 6 bits as a fragment id counter (little endian)
    • this give allows a maximal payload of 1512 bytes. (24bytes*63IDs)
  • use the last 2 bits either as a bit mask or a 2 bit number to define the fragments type
  • Example: header.fragment_id = 0b01011010 = 90 dec
    • the fragment ID is 0b00011010 = 26 dec
    • the fragment type is 0b01 = 1 dec (e.g. MORE_FRAGMENTS_FLAG)

I personally prefer the second option. If the user "above" the lib does not need to know much or change its code its a win situation. Although using this new schema means a maximal IP payload of 1500bytes. Anyways: jumbo frames should not be send across a NR24Network ^^

Does use someone the field header.next_id? The lib seems not to use this.

[TMRh20]

Haha, nice timing, I guess you were thinking about this issue too. After creating the error checking per your suggestions, I realized that the final payload can include the original header type, since we know we are expecting the final payload, we don't need the fragment_id, it can be inferred by the expected total_fragments count.

*Also the next_id is not used, not sure about its origins.

[reixd]

If we can restore the user type, we should do it. But we need to document this feature to not confuse other lib users/developers that it is not an error.

[TMRh20]

Good call there, it would probably confuse me if I looked at it next week. I can see about adding some comments etc. to be pushed on the next commit.