Net oddities

[WilliamAtSamraksh]

Continuing my review of eMote interfaces.

• Callbacks
  • Class is public but comes with a note that users should not access it. This can be confusing to users. Class should be internal.
• Exceptions
  • All of the exceptions only do Debug.Print. They do not actually throw an exception. Hence failure will be silent unless MFDeploy or the debugger is attached; and even if so, the failure can’t be trapped by the program. This isn’t very useful; actual exceptions would be a lot better.
  • Link is a struct, which is inefficient; likewise NeighborTable. With rare exceptions, all structs should be classes.
• Class MacConfiguration
  • In 3rd constructor, argument NeighborLivelinesDelay (sb NeighborLivenessDelay) is not used. public MacConfiguration(bool CCA, byte NumberOfRetries, byte CCASenseTime, byte BufferSize, byte RadioID, UInt32 NeighbourLivenessDelay, Radio.RadioConfiguration config) [Note: In my working copy I've Obsoleted members with Livelines and created new members with Liveness. Obsolete members can still be used.]
  • The number of constructors seems excessive. Are there use cases for each?
• Interface IMac:
  • UnInitialize returns DeviceStatus; SetAddress returns bool. Are these correct return values? Why not make them all DeviceStatus?
  • If GetAddress fails, does it throw an exception? Should SetAddress also do so?
  • GetNeighborStatus, GetBufferSize and GetPendingPacketCount are methods. It would make more sense to make them properties.
• Class IRadio
  • "Channels" enum should be singular (same for other enums)
• OMAC & CSMA namespaces. OMAC is in Samraksh.eMote.Net; CSMA is in Samraksh.eMote.Net.Mac. They should both be in the same namespace.
• Class MacBase
  • Why is Configure a static method? It seems to me that it should be a non-static method of MacConfiguration.
  • Method GetNextPacket returns a type of Samraksh.eMote.Net.Message. This is confusing. Packet & message should either be clearly distinguished, or one of the terms should be eliminated. This ambiguity shows up in a number of places (e.g. RemovePacket has an argument named msg) and is pretty confusing
• Class Message
  • Method IsSenderTimeStamped: shouldn’t it be IsMessageTimeStamped?
  • Method IsSenderTimeStamped: should be a property rather than a method.
  • Field Unicast should be IsUnicast. Probably should also be a property.
• Class Radio_802_15_4
  • What is the difference between GetInstance and GetShallowInstance? The latter doesn't seem to be shallow.
  • The methods seem to differ by argument. Why not overload GetInstance? Likewise Radio_802_15_4_LR.
• Class Radio_802_15_4_Base
  • Enum Channels should be Channel
  • Get* and Set* methods should be properties. It would reduce the number of methods and be more intuitive to the user. Suggest obsoleting these and creating corresponding properties.
  • RadioName should be RadioKind or RadioType or the like; it's not really a name.

[WilliamAtSamraksh]

Wrt the comment on exceptions: they are defined correctly. However, a Debug.Print is never a good idea in a released interop and should be removed.

[WilliamAtSamraksh]

Since the radio is in focus these days, can we get these issues addressed? Particularly annoying is the confusion between "packet" and "message" in the interface.

[WilliamAtSamraksh]

Another point ...

• CSMA has a GetRadio() method. This should be public read, private set property. So instead of CSMA.GetRadio()... it would be CSMA.Radio.... Generally, properties are appropriate when no state is being changed except backing variables.