[Bugs] #230 HIGH: No mesh support under 83.x (joyride 2631)

[Sugar Labs Bugs]

#230: No mesh support under 83.x (joyride 2631)
----------------------------+-----------------------------------------------
    Reporter:  garycmartin  |          Owner:  tomeu   
        Type:  enhancement  |         Status:  assigned
    Priority:  High         |      Milestone:  0.88    
   Component:  sugar        |        Version:  0.83.x  
    Severity:  Major        |       Keywords:  r!      
Distribution:  Unspecified  |   Status_field:  New     
----------------------------+-----------------------------------------------
Changes (by tomeu):

  * keywords:  r? => r!


Comment:

 Excellent, follow some comments, mainly about style:

 {{{
         self._palette.props.primary_text = _("Mesh Network") + " " +
 str(self._channel)
 }}}

 Wonder if in RTL scripts the number would make more sense to be on the
 left.

 {{{
             except dbus.exceptions.DBusException:
         605                     pass
 }}}

 We really don't want to log anything?

 {{{
     def __init__(self, device, tray, view_class=WirelessDeviceView):
 ...
         self._device_view = view_class(self._device)
 }}}

 Find this hard to read, cannot just pass the device type and instantiate
 one or the other with an if?

 {{{
         self._palette = self._create_palette()
         489             self.set_palette(self._palette)
 }}}

 Would be better to create the palette when it's needed (grep the code for
 "def create_palette"), in order to save memory.

 {{{
         p = palette.Palette(_("Mesh Network") + " " + str(self._channel))
 }}}

 We should try to avoid such identifiers, if each of us uses the smallest
 names that make sense to ourselves, others will end up having lots of
 difficulties.

 {{{
     def _disconnect_activate_cb(self, item):
 }}}

 Only one _? Guess you cannot remove this handler altogether?

 {{{
         self._greyed_out = (query != '')
 }}}

 Wonder if wouldn't make sense to accept the local translation of mesh for
 helping locating the mesh icons.

 {{{
 from jarabe.model.network import OlpcMesh as OlpcMeshSettings
 }}}

 Wouldn't be better to just name jarabe.model.network.OlpcMesh as
 OlpcMeshSettings?

 {{{
     def __init__(self, meshdev):
 }}}

 Can we call it mesh_device or just device? Even more as it's a public
 member.

 {{{
         self.ethdev = self._bus.get_object(_NM_SERVICE, ethdev_o)
 ...
         self._bus.add_signal_receiver(self.__mshdev_state_changed_cb,
 }}}

 Same as above.

 {{{
         # this is a stack of connections that we'll iterate through until
         # we find one that works
         self._connection_queue = []
 }}}

 Is it a stack or a queue?

 {{{
     # Activate a mesh connection on a user-specified channel
     # Looks for XS first, then resorts to simple mesh
     def user_activate_channel(self, channel):
 }}}

 Can we make this and other comments apidocs? Also, would be good to remove
 as many of the inline comments as possible, possibly by splitting some of
 the methods and then giving apidocs to those if the method names are not
 enough.

-- 
Ticket URL: <http://bugs.sugarlabs.org/ticket/230#comment:11>
Sugar Labs <http://sugarlabs.org/>
Sugar Labs bug tracking system