Opened 8 years ago

Closed 7 years ago

Last modified 4 years ago

#230 closed enhancement (fixed)

No mesh support under 83.x (joyride 2631)

Reported by: garycmartin Owned by: tomeu
Priority: High Milestone:
Component: Sugar Version: 0.83.x
Severity: Major Keywords:
Cc: eben, dsd, sascha_silbe Distribution/OS: Unspecified
Bug Status: New

Description

Testing an XO-1 under joyride 2631, the neighbourhood view and/or the device frame area show no UI related to the support mesh networking. I was vaguely expecting the 3 neighbourhood mesh icons to perhaps have been removed, and replaced with a single frame device mesh icon, with mesh controlling palette, not sure the status of this.

Attachments (4)

0001-Add-OLPC-mesh-support-230.patch (35.2 KB) - added by dsd 7 years ago.
0001-Add-OLPC-mesh-support-230.2.patch (35.2 KB) - added by dsd 7 years ago.
updated patch with improved device icon behaviour
0001-initial-review.patch (11.4 KB) - added by dsd 7 years ago.
actually its an incremental patch, hopefully makes review easier?
0001-Implement-mesh-support-again-dsd.patch (35.4 KB) - added by tomeu 7 years ago.
complete patch

Download all attachments as: .zip

Change History (19)

comment:1 Changed 8 years ago by garycmartin

OK, after some more testing with a 2nd XO...

Joyride 2631 neighbourhood view shows no mesh UIs, until another machine 'creates' a mesh network. I have a 2nd machine running 767, if both are connected to an AP, no mesh UIs appear on Joyride 2631. If I then switch the 767 XO to one of the mesh channels, the Joyride 2631 XO displays 2 (??) orange fill with green stroke circles in the neighbourhood view. Both are called olpc-mesh (no indication of channel). Clicking either seems to have no (successful) effect, i.e no mesh device in the frame and no sign of the other buddy when clicking (and waiting) either. A click does make the circle fill pulse, and changes the palette to "olpc-mesh, Connecting..., Disconnect" but reverts to just "olpc-mesh, Connect" after about ~25sec.

comment:2 Changed 8 years ago by marcopg

  • Bug Status set to New
  • Distribution/OS set to Unspecified
  • Milestone set to 0.86
  • Priority changed from blocker to High
  • Severity set to Major

I fear this is not going to get done by 0.84 :( It would be good to raise the issue on the laptop.org mailing lists to see if someone is interested to contribute it.

comment:3 Changed 8 years ago by garycmartin

I can't see mesh happening for 0.86, perhaps Link-Local/Ad-hoc networking will be enough so we at least have the infrastructure free 'kids under a tree' scenario still working for Sugar.

comment:4 Changed 8 years ago by tomeu

  • Cc dsd added

Daniel may be interested on this.

comment:5 follow-up: Changed 8 years ago by dsd

NetworkManager-0.8 from git supports OLPC mesh, now we just need sugar support again. here's a patch against 0.84.5

http://dev.laptop.org/~dsd/20090716/olpcmesh.patch

comment:6 Changed 8 years ago by sascha_silbe

  • Cc sascha_silbe added

comment:7 in reply to: ↑ 5 Changed 8 years ago by tomeu

Replying to dsd:

NetworkManager-0.8 from git supports OLPC mesh, now we just need sugar support again. here's a patch against 0.84.5

http://dev.laptop.org/~dsd/20090716/olpcmesh.patch

Thanks for the patch, please see http://wiki.sugarlabs.org/go/Development_Team/Code_Review for instructions.

comment:8 Changed 8 years ago by tomeu

  • Milestone changed from 0.86 to 0.88

Looks like this won't make 0.86 :(

comment:9 Changed 8 years ago by tomeu

  • Type changed from defect to enhancement

Changed 7 years ago by dsd

comment:10 Changed 7 years ago by dsd

  • Keywords r? added
  • Owner changed from marcopg to tomeu
  • Status changed from new to assigned

patch attached, please review

Changed 7 years ago by dsd

updated patch with improved device icon behaviour

comment:11 follow-up: Changed 7 years ago by tomeu

  • Keywords r! added; r? removed

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.

comment:12 in reply to: ↑ 11 Changed 7 years ago by dsd

  • Keywords r? added; r! removed

most of your comments apply equally to existing code found in those files, but i've made the changes for most of them.

Replying to tomeu:

We really don't want to log anything?

No - the connections might have changed state during the loop

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

I had a quick look and couldn't figure this out. The other 3 classes in this file create the palette the same way.

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

it would be nice but I haven't bothered to implement this this time around. The behaviour that I have implemented matches 0.82.

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

this would break the naming convention of classes in that file

Is it a stack or a queue?

its a connection queue implemented as a stack

I couldn't see any opportunities to divide functions up without impacting code readability.

sorry that I'm a bit strapped for time..here's an updated patch

Changed 7 years ago by dsd

actually its an incremental patch, hopefully makes review easier?

comment:13 Changed 7 years ago by tomeu

  • Keywords r! added; r? removed

actually its an incremental patch, hopefully makes review easier?

Better to always attach the whole patch, can you do so? I will finish it if you cannot devote more time on it.

Changed 7 years ago by tomeu

complete patch

comment:14 Changed 7 years ago by tomeu

  • Keywords r! removed
  • Resolution set to fixed
  • Status changed from assigned to closed

Pushed as http://git.sugarlabs.org/projects/sugar/repos/mainline/commits/2351cf86

Thanks a lot for tackling this long-standing issue.

comment:15 Changed 4 years ago by dnarvaez

  • Milestone 0.88 deleted

Milestone 0.88 deleted

Note: See TracTickets for help on using tickets.