Device Emulation (contd.): Checked and decided on a couple of FIXMEs/TODOs

binding/core.py

@@ -18,7 +18,7 @@
 import os
 from rpc import OstinatoRpcChannel, OstinatoRpcController, RpcError
 import protocols.protocol_pb2 as ost_pb
-import protocols.emulproto_pb2 as emul # FIXME: change name?
+import protocols.emulproto_pb2 as emul
 from __init__ import __version__
 
 class DroneProxy(object):

client/devicegroupmodel.cpp

@@ -200,7 +200,8 @@ bool DeviceGroupModel::setData(
     if (!port_)
         return false;
 
-    // FIXME
+    // TODO; when implementing also implement flags() to
+    // return ItemIsEditable
     return false;
 }
 

common/emulproto.proto

@@ -50,6 +50,9 @@ message MacEmulation {
     optional uint64 step = 10 [default = 1];
 }
 
+// No default values for IP addresses - user needs to explicitly set that
+// 'coz we derive if a device has a single/dual or no IP stack on the basis
+// of whether OstProto.DeviceGroup.ip[46] is set
 message Ip4Emulation {
     optional uint32 address = 1;
     optional uint32 prefix_length = 2 [default = 24];

common/protocol.proto

@@ -277,11 +277,6 @@ message Notification {
 
 /*
  * Protocol Emulation
- * FIXME: review/fix tag numbers
- * FIXME: move xxxEmulation to their own .proto files?
- * FIXME: What will be the contents of a default device created by addDeviceGroup()?
- * FIXME: decide default values for device and protoEmulations
- * FIXME: rename 'DeviceGroup'?
  * FIXME: merge getDeviceList() and get DeviceNeighbors() into a single
  *        getDeviceInfo() RPC?
  */

server/devicemanager.cpp

@@ -454,8 +454,6 @@ void DeviceManager::enumerateDevices(
                           ip6.prefix_length(),
                           UINT128(ip6.default_gateway()));
 
-            // TODO: fill in other pbDevice data
-
             switch (oper) {
                 case kAdd:
                     if (deviceList_.contains(dk.key())) {

server/packetbuffer.cpp

@@ -76,7 +76,6 @@ uchar* PacketBuffer::end() const
 
 void PacketBuffer::reserve(int len)
 {
-    // FIXME: add validation
     data_ += len;
     tail_ += len;
 }

server/pcapport.cpp

@@ -947,8 +947,14 @@ void PcapPort::EmulationTransceiver::run()
     char errbuf[PCAP_ERRBUF_SIZE] = "";
     struct bpf_program bpf;
 #if 0
+    const char *capture_filter =
+        "arp or icmp or icmp6 or "
+        "(vlan and (arp or icmp or icmp6)) or "
+        "(vlan and vlan and (arp or icmp or icmp6)) or "
+        "(vlan and vlan and vlan and (arp or icmp or icmp6)) or "
+        "(vlan and vlan and vlan and vlan and (arp or icmp or icmp6))";
 /*
-    Ideally we should use the below filter, but the 'vlan' capture filter
+    Ideally we should use the above filter, but the 'vlan' capture filter
     in libpcap is implemented as a kludge. From the pcap-filter man page -
 
     vlan [vlan_id]
@@ -965,12 +971,6 @@ void PcapPort::EmulationTransceiver::run()
     So we use the modified filter expression that works as we intend. If ever
     libpcap changes their implementation, this will need to change as well.
 */
-    const char *capture_filter =
-        "arp or icmp or icmp6 or "
-        "(vlan and (arp or icmp or icmp6)) or "
-        "(vlan and vlan and (arp or icmp or icmp6)) or "
-        "(vlan and vlan and vlan and (arp or icmp or icmp6)) or "
-        "(vlan and vlan and vlan and vlan and (arp or icmp or icmp6))";
 #else
     const char *capture_filter =
         "arp or icmp or icmp6 or "
@@ -989,7 +989,6 @@ void PcapPort::EmulationTransceiver::run()
 #endif
 
 _retry:
-    // FIXME: use 0 timeout value?
 #ifdef Q_OS_WIN32
     // NOCAPTURE_LOCAL needs windows only pcap_open()
     handle_ = pcap_open(qPrintable(device_), 65535,
@@ -1024,7 +1023,10 @@ _retry:
         }
     }
 
-    // FIXME: hardcoded filter
+    // TODO: for now the filter is hardcoded to accept tagged/untagged
+    // ARP/NDP or ICMPv4/v6; when more protocols are added, we may need
+    // to derive this filter based on which protocols are configured
+    // on the devices
     if (pcap_compile(handle_, &bpf, capture_filter, optimize, 0) < 0)
     {
         qWarning("%s: error compiling filter: %s", qPrintable(device_),