Fixes issue- Fails with Ubuntu 18.04

[Saumya-Mishra9129]

Pr should fix #6 . I have tested with Ubuntu 18.04.
In utils.py,(line 107) PangoCairo.create_layout method takes cairo.Context as argument. As cairo.Context argument is already given by RulerActivity.py in line 75. So there is no need to create context again in util.py(line 103).
@quozl Please review.

Fixes #10.

[chimosky]

Tested, I get this error.

Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/dbus/connection.py", line 607, in msg_reply_handler
    *message.get_args_list()))
  File "/usr/lib/python2.7/dist-packages/sugar3/activity/activity.py", line 838, in __save_error_cb
    err)
RuntimeError: Error saving activity object to datastore: org.freedesktop.DBus.Python.TypeError: Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/dbus/service.py", line 707, in _message_cb
    retval = candidate_method(self, *args, **keywords)
  File "/usr/lib/python3.7/dist-packages/carquinyol/datastore.py", line 365, in update
    self._metadata_store.store(uid, props)
  File "/usr/lib/python3.7/dist-packages/carquinyol/metadatastore.py", line 25, in store
    self._set_property(uid, key, value, md_path=metadata_path)
  File "/usr/lib/python3.7/dist-packages/carquinyol/metadatastore.py", line 63, in _set_property
    f.write(value)
TypeError: a bytes-like object is required, not 'dbus.Double'

[Saumya-Mishra9129]

@chimosky Tested the above error has gone. Review again.

[Saumya-Mishra9129]

/usr/lib/python2.7/dist-packages/sugar3/activity/activityinstance.py:59: Warning: 
    g_hash_table_destroy: assertion 'hash_table != NULL' failed
  activity = constructor(handle)
Normal successful completion, pid 2510 activity_id 037583e93333338a398c889959ab41dc55r1d2389

Getting this warning , but activity is running normally.

[quozl]

Thanks. Reviewed.

• most other activities use cr rather than c to hold the Cairo Context,
• the earlier port to GTK 3 did not rename the draw callback properly, so the name and arguments to the __expose_event_cb don't look right, and a new Cairo Context is being used instead of the context provided by the draw signal; this may explain the cause, (edit; is also tracked as Maintain consistency with draw signal callback #10),
• cf10e9f doesn't look correct; we have more than 300 examples of not having to convert metadata, so please explain why you think this has to be done,

Suggest stepping back and looking for why these changes were necessary, and therefore if there are other changes that will be required.

Tested on Ubuntu 20.04;

• there's a redraw of the canvas when stop is pressed; possibly related to the misuse of the Cairo Context,
• the canvas redraw seems to stutter; possibly same cause,
• the rulers do not fit on the 1024x768 pixel resolution of my virtual machine.

Fixes #6.

Evaluated the usefulness of the activity in changed circumstances, and created issue #11.

[Saumya-Mishra9129]

@quozl cf10e9f is wrong. The correct solution will be converting new to str as new is assigned to MMPERINCH in method [def custom_unit_change_cb(self, widget)](https://github.com/sugarlabs/ruler/blob/master/RulerActivity.py#L264). It should be converted to str because new is a float. Still getting NotImplementedError as warning in self.read_file(self._jobject.file_path). Look Below seems like the activity is not complete.

most other activities use cr rather than c to hold the Cairo Context, I have searched many activities uses cr, but it require changes in whole activity.
I have removed __expose_event_cb with __draw_cb. It should fix issue #10 .

[Saumya-Mishra9129]

The activity is not yet ported to python 3. I should port it also.

[Saumya-Mishra9129]

there's a redraw of the canvas when stop is pressed; possibly related to the misuse of the Cairo Context,
I was also getting this error before 49b2d3a . But after that fix I am not getting that error.
the rulers do not fit on the 1024x768 pixel resolution of my virtual machine.
I am also facing this problem. I have to look at the right cause of getting this one.

[quozl]

I've diagnosed the cause of the g_hash_table_destroy error. It is caused by calling GdkX11.X11Screen, which is no longer a supported method for finding the display resolution. Instead we now use GTK directly.

For interest, the method I used was;

• add temporary debugging to activityinstance.py in the toolkit, see diff; this told me the error was asynchronous, happening immediately after the activity constructor, and before the return to create_activity_instance,
• add debugging and iterate to remove features from the activity, see diff, until the g_hash_table_destroy error vanishes, and then restoring and marking the code that was the trigger.

Then to verify the theory, I took the Hello World activity source code, and applied a similar change, see diff, and proved that the g_hash_table_destroy assertion begins to occur.

[quozl]

Thanks. Reviewed. I've added two commits;

• 8cf1d3e ("Review - fix style and indentation"),
• 8348297 ("Review - metadata custom_unit has changed type"),

So this pull request seeks to change the data type for the custom_unit metadata.

I'm worried about upgrade path. Here's the scenario; a child has been using the released version of Ruler, and so their Journal may contain objects associated with the activity containing floating point_ values for custom_unit, and here's where it will go wrong;

• child uses the old Ruler activity,
• child upgrades, or has upgraded for them, the Ruler activity,
• child starts the Ruler activity.

Choices are to either;

• find out why the Datastore isn't able to store floating point numbers, in turn why D-Bus isn't able to transmit them, and go back to using floating point in Ruler, or;
• make the Ruler activity resilient to bad metadata, by careful testing.

I'm worried this is a defect in Datastore that hasn't been resolved since we ported to Python 3. The Python 2 release of Datastore had no such problem, as shown by how Ruler was blithely placing floating point values in metadata.

[Saumya-Mishra9129]

I'm worried this is a defect in Datastore that hasn't been resolved since we ported to Python 3. The Python 2 release of Datastore had no such problem, as shown by how Ruler was blithely placing floating point values in metadata.

We need to carefully test the port to Python3 of Datastore. As shown by the activity, a careful testing is needed in Datastore.

make the Ruler activity resilient to bad metadata, by careful testing.

This should be done according to me , We need to fix issues in datastore, in order to fix the issue of storing floating point values in metadata.

[Saumya-Mishra9129]

I've diagnosed the cause of the g_hash_table_destroy error. It is caused by calling GdkX11.X11Screen, which is no longer a supported method for finding the display resolution. Instead we now use GTK directly.

For interest, the method I used was;

• add temporary debugging to activityinstance.py in the toolkit, see diff; this told me the error was asynchronous, happening immediately after the activity constructor, and before the return to create_activity_instance,
• add debugging and iterate to remove features from the activity, see diff, until the g_hash_table_destroy error vanishes, and then restoring and marking the code that was the trigger.

Then to verify the theory, I took the Hello World activity source code, and applied a similar change, see diff, and proved that the g_hash_table_destroy assertion begins to occur.

Thanks. Your method of diagnosing looks great. I have checked for GdkX11.X11Screen, the class exists but it does not have methods like width() and height(). I guess it was creating problem. Tried to fix the issue in f0a0035 using Gdk.Screen class.

[quozl]

Thanks. Your method of diagnosing looks great. I have checked for GdkX11.X11Screen, the class exists but it does not have methods like width() and height().

Huh. Interesting. On my system an instance of the class certainly does have these methods, and they can be called, and give good results, apart from that apparently harmless error. Python's help also correctly shows them. Documentation for PyGObject does not show them. Exiting from Python REPL after using GdkX11.X11Screen() does generate the assertion error.

I guess it was creating problem. Tried to fix the issue in f0a0035 using Gdk.Screen class.

Thanks. Yes, it does fix this error.

[chimosky]

@Saumya-Mishra9129 please avoid merge commits as it makes commit history a bit difficult to look at, you can do a rebase instead. Thanks.

[Saumya-Mishra9129]

the rulers do not fit on the 1024x768 pixel resolution of my virtual machine.

@quozl I have tried to fix resolution of ruler in 3870323.

[quozl]

Thanks. I've reviewed 3870323 only. It is difficult to understand, and took me a while, because the commit message was so brief, and an unusual variable L was used which I mistook for a 1. It looks like you are solving this for 1024x768 pixel resolution only, by scaling the width of the ruler by nett width less one twentieth nett width.

However, on testing I see a completely different effect. Here is c4a9ea8;

note the cm and inch rulers extend beyond the display on either side.

and here is 3870323;

note the cm and inch rulers extend beyond the display on the right hand side.

Keep in mind that the OLPC XO for which this activity was written had a display resolution of 1200x900 which could be rotated to 900x1200.

[Saumya-Mishra9129]

@quozl Review and merge it.

note the cm and inch rulers extend beyond the display on the right hand side.

It looks like you are solving this for 1024x768 pixel resolution only, by scaling the width of the ruler by nett width less one twentieth nett width.

Hoping 05cdcf4 will fix this . Yes I am trying this on 1024x768 pixel resolution but It will work on OLPC XO as well.

[chimosky]

Tested 05cdcf4 on a 1366x694 resolution and this was the result; the right hand side still extends beyond the display.

[Saumya-Mishra9129]

Tested 05cdcf4 on a 1366x694 resolution and this was the result; the right hand side still extends beyond the display.

Can we really fix the right side display?. According to me, we cannot, as custom ruler is of variable size, it is set 1 cm equals to 24.5 mm by default, and we can go to settings and modify it accordingly , so what is the need to fix it initially, if we can change it according to our need.

[chimosky]

Can we really fix the right side display?. According to me, we cannot, as custom ruler is of variable size, it is set 1 cm equals to 24.5 mm by default, and we can go to settings and modify it accordingly

Depends, we can fix it by limiting it to a particular length; also I see that the default is 25.4 mm or is that not the case?

[quozl]

Thanks. Tested. Checked items above that I had observed.

• not a problem that the ruler stops prematurely; just that it shouldn't seem to vanish off the edge,
• the 25.4mm is presumably an inch, so it can be kept.