TEP 119 comments
1.) There is a code snippet of LinkEstimator interface in section 4.1, but it doesn't match the latest TinyOS tree. Specifically, it is missing:
command uint16_t getReverseQuality(uint16_t neighbor); command uint16_t getForwardQuality(uint16_t neighbor);
(Ed: This is an example of the issues generated by including implementation specific information that should be discussed in TEP 123 (124 ?) in this TEP. A better way would be to separate the two)
These interfaces no longer exist in the CVS head for 2.1. This is a reminder to set TinyOS-Version to 2.1
2.) The second sentence of 4.3 ("... outcome of attempted transmissions.:") should just have ":", not ".:"
:: is a special symbol not a typo
3.) Section 4 talks about the T2 CTP implementation. Since net2 has recently reserved AM IDs for CTP, it might be helpful to include this info. However, I am also debating whether this info is suitable only for TEP 123. (Ed: This comment is similar to the discussion about removing the CTP part and having only a reference to TEP 123)
We need to describe the implementation but should it be shorter?
4.) Is (near) real-time delivery a requirement for collection?
I don't think so
1. It would be nice to mention the default AMIDs that are reserved for CTP. After all, it did change very recently.
2. In section 2, dividing the elements as 'producer, snooper, in-network processor, and consumer' is not straightforward enough. Although explanations follow this, it might be better to give straightforward names like 'root' and 'source' instead of 'consumer' and 'producer'.
(Ed: It might be a good idea to have a 'terminology' section in the beginning as many RFCs do)
3. In section 4, although not further maintained, it might be good to explicitly say that tos/lib/net/le/ can be used as a substitute of tos/lib/net/4bitle/. Or the folder can be deleted from the tree for less confusion.
(Ed: Are the two interchangeable or does '4bitle' replace 'le'?)
This seems a comment on the code rather than TEP. le and 4bitle are interchangeable.
4. Typo in 4.1. RSI should be changed to RSSI.
5. In section 2, in the Intercept part, the TEP should talk about the preferred timing the Intercept should happen. In other words, it would be nice to explicitly say that the Intercept.forward() SHOULD happen before the packet is preserved in the sending queue. This is currently how the forwarding engine is designed.
I don't think it's a good idea to put a SHOULD in a document based on how something is currently implemented. There could be very good reasons for doing otherwise. -pal
Also in section 2, the part, "This interface allows a higher layer to inspect the internals of a packet and possibly suppress it if it is unnecessary or if its contents can be aggregated into an existing packet"
There are many other jobs you can do with a intercepted packet and SHOULD NOT be limited to such examples. Additionally it would be nice to note that the ownership of the packet belongs to the CTPForwardingEngine. Also note the fact that the signaled higher layer should copy the data to mangle with the message.
I added a sentence about copying and ownership. I also changed the text to suggest that there are other uses. -pal
6. In section 3, the following part, "ColletionC MUST NOT signal Receive.receive on non-root nodes. CollectionC MAY signal Receive.receive on a root node when a data packet successfully arrives at that node."
For the second sentence, I believe "CollectionC SHOULD signal Receive.receive on a root node when a data packet successfully arrives at that node" is the correct way of saying it. Because CTP will mostly be used in cases to collect data, signaling Receive.receive SHOULD be the right thing to do at the root.
I think I agree. -om
I'm on the fence on this one. I can imagine lots of collection protocols that don't signal receive because they do other things with the packets. Older collection protocols (1.x) actually just forwarded to the UART automatically. It's clear that implementations today signal Receive, but it's perfectly reasonable to have one that doesn't. -pal
I don't see why calling receive would restrict you from doing anything. You can easily implement 'forward to the UART' in the receive handler. MAY is still fine, though. -rodrigo
7. In section 3.1, more explanation on the characteristics of the Packet interface would be useful. The Packet interface provided from CTP takes encounter the size of the CTP header. Therefore, it would be nice to say that to send a packet using CTP, the user SHOULD use the Packet interface provided by CollectionSenderC to retrieve/set packet related information. (e.g. getPayload, setPayloadLength will return/set values with respect to the length of the CTP header).
This kind of information is covered in 116. -pal
-In the abstract, "Collection provides best-effort multihop delivery..."
-Wondering whether it would be better (at least in the abstract) to keep the description at a high level, replacing "roots" with "collection points". The reason for doing this is that at a high-level the collection service should not specify whether the implementation should use a tree or some other topology to deliver the data. The only thing required is that the data is delivered to one or more pre-determined collection points.
-In the Introduction, "When a network has multiple base stations that act as *root* nodes" is confusing. Can there be base stations that do not act as roots? You can instead say "When a network has multiple roots"
-In the Introduction, "a collection protocol implicitly joins" should be replaced with "a node participating/implementing the collection protocol implicitly joins"
-In the introduction, "There are also no ordering guarantees"
-In the introduction, "simple collection protocols encounter several challenges". Should "simple" go away? Otherwise what is the definition of simple?
-In the same paragraph, what does "this protocol family" refer to?
-"Duplicate suppression, detecting and dealing with lost acknowledgments that can cause packets to replicate..."
-Section 2, "The nodes can process a packet that is in transit"
-"The Intercept interface has this signature:" (remove second colon) Likewise for the RootControl interface
:: is a special symbol
-In Section 2, can unsetRoot()" return FAIL?
-In Section 3, the description that Receive.receive MAY be signaled when a packet arrives at the root is somewhat surprising. At the very least provide some explanation why a signal could not be generated even though the packet successfully arrived at the root. Is it a buffering issue?
-In Section 3, "so that snoopers, intercepters, and receivers can parse them properly"
-In Section 3, "If CollectionC receives a data packet that a different node is supposed to forward, it MAY signal Snoop.receive". Is this the only case when Snoop.receive will be generated? In other words, it is not generated for packets that pass through a node? These semantics are different from a regular snooper which receives *all* packets
-Regarding Section 4, I think it should be removed completely and be replaced only with a reference to TEP 123. The reason for doing so is that it keeps the interface document (TEP 119) clean and avoids mismatches and having to update two documents when the prototype implementation changes.
done, which renders the comments below not applicable to this document.
-Nonetheless, below are some comments related to this section.
-Reference to ETX is needed.
-The concept of the neighbor table is used but it is not introduced.
-My understanding from the text is that only information from explicit beacons (i.e. packets sent using the LinkEstimatorP.Send interface) can be used to update the neighbor table. What about routing protocols that use information from piggybacked to data packets?
The reference CTP implementation uses information about data transmission statistics to update the neighbor table but collection does not provide generic mechanism for layers to piggyback information and use that information at the receiving end.
-Can both the LinkEstimatorP and routing engine insert entries in the neighbor table?
-In the LinkEstimator interface, imagine that an upper layer inserts an entry using the neighbor's am_addr_t. How can it later figure out which entry in the neighbor table corresponds to that neighbor?
-In Section 4.2, "this is part of the control plane of the network, and it does not directly"