TelepathyGLib Port

[srevinsaju]

This is a TelepathyGLib Port for TurtleArts, which was not ported. And No Issue opened for it

• [x] Port initially to TelepathyGLib
• [x] Port to Python3 for non-ported files
• [x] Fix Deprecation
• [x] Fixed Known Issues: turtleblocks fails
• [ ] Test Collaboration

[srevinsaju]

@quozl I have successfully ported to TelepathyGlib, python3 and tested on GNOME and KDE environment. On importing textchannelwrapper.py into the turtleblocks file, it raises the following error:

[Errno 2] No such file or directory: '/home/srevinsaju/.config/turtleart/turtleartrc.collab'
Traceback (most recent call last):
  File "/home/srevinsaju/Totru/turtleart-activity/gnome_plugins/collaboration_plugin.py", line 94, in _connect_cb
    self.setup()
  File "/home/srevinsaju/Totru/turtleart-activity/gnome_plugins/collaboration_plugin.py", line 98, in setup
    self._collaboration.setup()
  File "/home/srevinsaju/Totru/turtleart-activity/TurtleArt/tacollaboration.py", line 66, in setup
    self.collab = CollabWrapper(self._activity)
  File "/home/srevinsaju/Totru/turtleart-activity/TurtleArt/textchannelwrapper.py", line 167, in __init__
    self.shared_activity = activity.shared_activity
AttributeError: 'Collaboration_plugin' object has no attribute 'shared_activity'

It seems like the new collabwrapper is not compatible with the one which wasn't ported earlier, but I am not sure though. However the GNOME, KDE application works good. To test it:

• cd to the turtleart-activity dir
• chmod +x ./turtleblocks
• ./turtleblocks
• Alternatively, use the *.desktop file, but I haven't tested that, but it should work

Some screenshots for your perusal, (Python programmiing seems easy after using turtleblocks :joy: )

I basically don't know all these blocks are, but they look cool. It needs great expertise to test it and merge it :crossed_fingers: I will fix deprecation errors soon

[srevinsaju]

Fixed deprecation on https://github.com/sugarlabs/turtleart-activity/pull/73/commits/1dafe817ff0911df87b3e089350d3caa1f97b501

[quozl]

Thanks, good progress.

• [x] _shared_activity changed to shared_activity some years ago in the Sugar Toolkit, but it appears this change hasn't been added to gnome_plugins/collaboration_plugin.py, so you should make the change there.
• [ ] please move the calls to require_version into the main program turtleblocks, as had already been done with the Sugar activity, otherwise there will be redundant calls,
• [x] please test with the .desktop file,
• [ ] please test collaboration; and describe how we can test it as well. Otherwise there's no purpose to having it in the repository.

[srevinsaju]

@quozl is there a updated repository having the new gnome_plugins

[srevinsaju]

Error so far:

[srevinsaju@localhost turtleart-activity]$ ./turtleblocks     
Successfully importing Accelerometer
Failed to load Audio_sensors: No module named 'numpy'
Successfully importing Camera_sensor
Successfully importing Light_sensor
Successfully importing Rfid
Successfully importing Turtle_blocks_extras
Traceback (most recent call last):
  File "/home/srevinsaju/Totru/turtleart-activity/gnome_plugins/collaboration_plugin.py", line 188, in _connect_to_neighborhood
    self._neighborhood = get_neighborhood(params)
  File "/home/srevinsaju/Totru/turtleart-activity/collaboration/neighborhood.py", line 1043, in get_neighborhood
    _neighborhood = Neighborhood(params)
  File "/home/srevinsaju/Totru/turtleart-activity/collaboration/neighborhood.py", line 657, in __init__
    self._buddies = {None: get_owner_instance()}
  File "/home/srevinsaju/Totru/turtleart-activity/collaboration/buddy.py", line 189, in get_owner_instance
    _owner_instance = OwnerBuddyModel()
  File "/home/srevinsaju/Totru/turtleart-activity/collaboration/buddy.py", line 121, in __init__
    self._connection  = Connection.new(bus_object, service, path)
TypeError: argument dbus: Expected TelepathyGLib.DBusDaemon, but got dbus.proxies.ProxyObject
ERROR:dbus.service:Unable to append (None,) to message with signature v: <class 'TypeError'>: Don't know which D-Bus type to use to encode type "NoneType"

[srevinsaju]

• [x] _shared_activity changed to shared_activity some years ago in the Sugar Toolkit, but it appears this change hasn't been added to gnome_plugins/collaboration_plugin.py, so you should make the change there.

• [ ] please move the calls to require_version into the main program turtleblocks, as had already been done with the Sugar activity, otherwise there will be redundant calls,

I would prefer it not to be called in turtleblocks, the reason is that, if I add gi.require_version() in the respective files, it will easily help in the porting of TortugadeMexico and other TurtleArts dependant actvity

• [x] please test with the .desktop file,

The turtleblocks.desktop file corresponds to python3 TurtleArt/turtleblocks.py. To test turtleblocks.desktop should be added to PATH, this can be done manually or by doing pip install turtleblocks

• [ ] please test collaboration; and describe how we can test it as well. Otherwise there's no purpose to having it in the repository.

I am trying to fix Collaboration on GNOME and KDE desktop. It seems to import sugar3.Activity but in the GNOME version, it does not include sugar3.Activity for obvious reasons, but it complains of the error after commit https://github.com/sugarlabs/turtleart-activity/pull/73/commits/5a7370240969fe951ea6f0a133c4a258517308ce and https://github.com/sugarlabs/turtleart-activity/pull/73/commits/a6218232b6919aba7c795ce59aaa0a3d843a1034 . The error is mentioned here https://github.com/sugarlabs/turtleart-activity/pull/73#issuecomment-566082788.

How to test Collaboration on GNOME, KDE TurtleBlocks

• start turtleblocks by executing

  chmod +x turtleblocks
  ./turtleblocks

  1. [x] Go to Titlebar Menu > Neighborhood > Enable Collaboration

     The turtleblocks is likely not to stop working, if it continues. In my case, it gives an error with no collaboration working, because the console gives the above output

  2. [ ] Go to Titlebar Menu > Neighborhood > Configuration

     Enter necessary details, in my case, I didn't know what to enter, so I entered my IRC details

  3. [ ] Test Collaboration by starting it on an alternative VM or another linux machine on the same network

NOTE:

• Possible solution, if turtleblocks collaboration is unable to be fixed, is to remove collaboration for only GNOME, KDE and preserve collaboration as a feature for only sugar desktops
• Virtual Machines are necessary only after completing Step 1 and Step 2. Currently I can do step 1 and Step 2 , but errors confirm that collaboration is not activated

Maintaining TurtleArt and turtleblocks is necessary as TurtleArt based activities like TortugadeMexico depends directly on this.

[quozl]

Thanks for the work so far. @srevinsaju wrote:

is there a updated repository having the new gnome_plugins

Sorry, I don't understand

Error so far: TypeError: argument dbus: Expected TelepathyGLib.DBusDaemon, but got dbus.proxies.ProxyObject

Either pass a DBusDaemon, or follow our Python 3 Porting Guide which says to replace calls to Channel and Connection classes of telepathy-python with a dictonary of dbus.Interface(). We found out earlier this year that to mix API layers caused us problems, so Sugar Toolkit, CollabWrapper, and other components have been migrated from the static telepathy binding (import telepathy) and the introspection binding (from gi.repository import TelepathyGLib), to using the D-Bus API aka Telepathy D-Bus Interface Specification, with the TelepathyGLib only used for string constants.

I would prefer it not to be called in turtleblocks, the reason is that, if I add gi.require_version() in the respective files, it will easily help in the porting of TortugadeMexico and other TurtleArts dependant actvity

I don't think this is enough of a benefit.

I would prefer it to be called at the start of every main program, so that;

• the warnings are suppressed, (the warnings cost I/O latency which slows laptops),
• change is minimised,
• the modules are probed in an efficient order,

Other dependent activities are typically maintained by forking and minimising the change, and when the fork is to be updated the branch is recreated with the change reapplied. @Kiy4h.

[srevinsaju]

Either pass a DBusDaemon, or follow our Python 3 Porting Guide which says to replace calls to Channel and Connection classes of telepathy-python with a dictonary of dbus.Interface(). We found out earlier this year that to mix API layers caused us problems, so Sugar Toolkit, CollabWrapper, and other components have been migrated from the static telepathy binding (import telepathy) and the introspection binding (from gi.repository import TelepathyGLib), to using the D-Bus API aka Telepathy D-Bus Interface Specification, with the TelepathyGLib only used for string constants.

Ok, i will try my best to create the dictionary thingy, I will do appropriate changes right now

I would prefer it to be called at the start of every main program, so that;

• the warnings are suppressed, (the warnings cost I/O latency which slows laptops),
• change is minimised,
• the modules are probed in an efficient order,

@quozl I am fine if its called in turtleblocks and other modules in TurtleArt, then I think, both TortugadeMexico and TurtleBlocks would benefit together. Is it fine, or should I add the require_version only in turtleblocks.py?

[quozl]

I'm not sure. Show me?

[quozl]

Please rebase.

[srevinsaju]

@quozl, @Kiy4h had cherry picked the needed commits, possibly which was needed. You can close this PR now :) if nothing is left from what @Kiy4h had done

[kiy4h]

I agree with @srevinsaju, since I've made sure to cherry-pick all the required commits. Some of the changes made here were part of port to python3, but I've applied that changes to another PR which has been merged.

[quozl]

Something's wrong. Please check?

git diff 2ba74a1..a621823

First commit is ta/master, second commit is this pull request. Some of those changes are what @Kiy4h has done in the meanwhile, but some seem right to accept anyway; e.g.

@@ -308,7 +306,7 @@ class GstPlayer(GObject.GObject):
 class VideoWidget(Gtk.DrawingArea):

     def __init__(self):
-        GObject.GObject.__init__(self)
+        Gtk.DrawingArea.__init__(self)
         self.set_events(Gdk.EventMask.EXPOSURE_MASK)
         self.imagesink = None
         self.set_double_buffered(True)

Calling Gtk.DrawingArea is more correct, how else would the Gtk.DrawingArea initialiser be called?

@@ -1360,14 +1360,14 @@ class TurtleArtWindow():
         if self.copying_blocks or self.sharing_blocks or self.saving_blocks:
             if blk is None or blk.type != 'block':
                 self.parent.get_window().set_cursor(
-                    Gdk.Cursor.new(Gdk.CursorType.LEFT_PTR))
+                    Gdk.Cursor(Gdk.CursorType.LEFT_PTR))
                 self.copying_blocks = False
                 self.sharing_blocks = False
                 self.saving_blocks = False
         elif self.deleting_blocks:
             if blk is None or blk.type != 'proto':
                 self.parent.get_window().set_cursor(
-                    Gdk.Cursor.new(Gdk.CursorType.LEFT_PTR))
+                    Gdk.Cursor(Gdk.CursorType.LEFT_PTR))
                 self.deleting_blocks = False
         if blk is not None:
             if blk.type == 'block':

Calling Gdk.Cursor.new is what we do in the toolkit.

[Saumya-Mishra9129]

@quozl

Calling Gtk.DrawingArea is more correct, how else would the Gtk.DrawingArea initialiser be called?

I reviewed, and I agree , as super class constructor must be called, - GObject.GObject.__init__(self) , this is the correct way to do this.

Calling Gdk.Cursor.new is what we do in the toolkit.

I found something when I searched for Gdk.Cursor, Gdk.Cursor.new is Deprecated since version 3.16: Use Gdk.Cursor.new_for_display() instead.. It seems like we have to make changes.

[quozl]

@quozl

Calling Gtk.DrawingArea is more correct, how else would the Gtk.DrawingArea initialiser be called?

I reviewed, and I agree , as super class constructor must be called, - GObject.GObject.__init__(self) , this is the correct way to do this.

I disagree. Calling GObject.GObject.__init__ will not execute Gtk.DrawingArea.__init__, and so anything the parent class author wants to happen is skipped. Let me know if you can demonstrate my assertion as incorrect.

Calling Gdk.Cursor.new is what we do in the toolkit.

I found something when I searched for Gdk.Cursor, Gdk.Cursor.new is Deprecated since version 3.16: Use Gdk.Cursor.new_for_display() instead.. It seems like we have to make changes.

We may make changes. We don't have to. GTK 3.16 was released in 2015, and as they haven't actually removed it yet I see no reason to make such a large change. We like to keep the widest compatibility with GTK so that older systems and newer systems can run the same code.

[Saumya-Mishra9129]

I disagree. Calling GObject.GObject.init will not execute Gtk.DrawingArea.init, and so anything the parent class author wants to happen is skipped. Let me know if you can demonstrate my assertion as incorrect.

Yes you are correct. My bad, I should have tried running first before claiming.

We may make changes. We don't have to. GTK 3.16 was released in 2015, and as they haven't actually removed it yet I see no reason to make such a large change. We like to keep the widest compatibility with GTK so that older systems and newer systems can run the same code.

Yess now making changes at this point of time is not a right. But we can still reserve it for future.

[srevinsaju]

@quozl I have fixed the merge conflicts now; In case if thats necessary to continue this PR or to close if its not required.

[chimosky]

@srevinsaju can you avoid a merge commit?

[srevinsaju]

Ok

[srevinsaju]

@chimosky Removed the merge commit

[quozl]

@srevinsaju, would you like to resolve conflicts?

[srevinsaju]

@quozl resolved; please forgive the merge commit

[srevinsaju]

Removed the merge commit too

[quozl]

We've lost something along the way.

When I look at the aggregate change of this pull request there are some flake8 changes, but no changes relating to TelepathyGLib porting.

When I git diff master..a6dd1a6, there is just a blank or line ending change.

The work you've done has vanished from the head of the branch. a131c8d223e167781314c4202d6d3c4f872b929b still has it, I think.

[srevinsaju]

Ok, well I do not have that much expertise to remove the merge commit. I recovered my commits from reflog, I had hard rebased the commits on master to remove the merge commit; my bad

[quozl]

Thanks.

I briefly reviewed your changes from 6666318 to a621823, and formed a view of what you were changing and why. The commit messages were very brief.

I tried a checkout of a621823 to rebase against current master df33738c65b5bc08c7671d615869a2078c64db44, but the conflicts were extensive and I felt I didn't have enough understanding of your changes to figure out how to resolve the conflicts by hand.

I saw that the only files you were changing were those in the collaboration directory.

I tried a recursive diff (using meld) of just the collaboration directory between a6218232b6919aba7c795ce59aaa0a3d843a1034, and that gave me a much better idea of what you intend.

I found collaboration/dispatch is from sugar-toolkit-gtk3.

I took the current master, then began to add changes from a6218232b6919aba7c795ce59aaa0a3d843a1034 collaboration directory;

• skipped whitespace and flake8 changes,
• skipped the GObject.SignalFlags.RUN_FIRST changes; both constants are available,
• skipped the % to format changes; % does still work,

resulting in 6cb2f3a8. Is this the change you are proposing now?

[srevinsaju]

@quozl along with https://github.com/sugarlabs/turtleart-activity/commit/6cb2f3a866153f3360a88b9852b2e4c5945b96a4, this may need to be included too https://github.com/sugarlabs/turtleart-activity/pull/73#discussion_r426426082

The aggregate changes may seem small, because some of my commits were cherry picked by @Kiy4h during GCI, hence the diff looks small except for some flake8 and format changes which I did during GCI, (I am not sure why I did that, but yea)

[quozl]

Okay, thanks. Well, if https://github.com/sugarlabs/turtleart-activity/commit/6cb2f3a866153f3360a88b9852b2e4c5945b96a4 is what you want, plus the other change you highlighted, perhaps you could

• reset your branch to 6cb2f3a866153f3360a88b9852b2e4c5945b96a4,
• make the additional change as a commit,
• push.

[srevinsaju]

Ok.

[srevinsaju]

@quozl Done.

Let me know if I can anything more.

[quozl]

Are you sure about https://github.com/sugarlabs/turtleart-activity/pull/73/commits/11664cd865577ce78232911ec8f70f0e7ab33420 and the need for both exceptions to be checked for? One is a subclass of the other, e.g.

>>> x = ModuleNotFoundError()
>>> isinstance(x, ImportError)
True
>>> █

[srevinsaju]

@quozl Thanks for that; A new thing for me; I am not sure I added that during GCI, but it was probably for a specific reason. I will drop https://github.com/sugarlabs/turtleart-activity/pull/73/commits/11664cd865577ce78232911ec8f70f0e7ab33420 for time being

[quozl]

Thanks. I'll review now.

[quozl]

Thanks. How is this to be tested? I'm not familiar with running Turtle Art (the application).

[srevinsaju]

I have found another bug, I am pushing the changes.

I have provided an explanation here. There is a turtleart.desktop which needs to be run outside sugar environment

[srevinsaju]

@quozl I guess I am done, I assume something is still missing. Please let me know after your testing. This is because I am not able to get a good coverage percentage, indicating some files remain still untested

[srevinsaju]

There is also a guide on https://github.com/sugarlabs/turtleart-activity/pull/73#issuecomment-565744341

[quozl]

It's okay, I was only curious. I won't test. My focus is Sugar. Thanks for your work on this.

[srevinsaju]

I guess this PR does not restore turtleart desktop version to the working condition after port to python3, but it should be relatively an improvement over the current implementation :smile:

[quozl]

I don't see any issues created. If you're aware of any that you don't have plans to fix, please create them.

[srevinsaju]

No, I am not aware of any issues yet. :)

[quozl]

I guess I don't understand. You say that this PR does not restore the application to working condition, but you don't know for sure either?

[srevinsaju]

I am sure, because I have not tested it completely; I have not reached a good coverage.

I have a strong feeling, because I just found a bug on https://github.com/sugarlabs/turtleart-activity/pull/73/commits/187d84dfe30487e80ab7a24a60c567b08d779efd few moments before the merge. So.. you know;