[WIP] Ofono bridge #83

monsieurh · 2018-01-13T16:36:37Z

ofono bridge progress

purpose

Use ofono as a backend for all phone/texting features

implementation

A new package ofono contains all the libraries, the class OfonoBridge is the interface between ZPUI and Ofono.
OfonoBridge uses D-Bus to communicate with ofono and thus needs a new dependency : pydbus library (added to requirements.txt).

issues

powering the modem down prevents from powering it back up afterwards

todo

zpui

will probably need its own thread to run the D-BUS loop
GUI for phone calls
GUI for texting
GUI for contact management

ofono

system

compile and distribute ofono binaries
distribute the default config file 00-ofono.rules for SIM800L (to be placed in /etc/udev/rules.d/)
make and distribute a systemd unit file to run ofono on startup

adds `import_sim_phonebook` to `OfonoBridge`

CRImier · 2018-01-13T16:49:29Z

ofono/bridge.py

+
+from helpers import Singleton, setup_logger
+
+DBusGMainLoop(set_as_default=True)  # has to be called first


threads_init() Initialize threads in dbus-glib, if this has not already been done. This must be called before creating a second thread in a program that uses this module.

(from here)

I wonder if this applies to us. If so, that's a big problem since many threads are created before app import even starts

CRImier · 2018-01-13T16:51:26Z

apps/personal/contacts/vcard_converter.py

@@ -1,5 +1,4 @@
-from helpers import setup_logger


Wait, why? I think you need to revert this particular change, which reverses our "setup_logger" integration work

(as in, revert the logger-> logging replacements, not the from_string addition)

CRImier · 2018-01-13T16:52:24Z

00-ofono.rules

@@ -0,0 +1,2 @@
+KERNEL=="ttyAMA0", ENV{OFONO_DRIVER}="sim900"


We'll be shipping this file in our own Debian package, most likely

CRImier · 2018-01-13T16:58:15Z

ofono/bridge.py

+        manager = bus.get('org.ofono', '/')
+        modem_path = manager.GetModems()[0][0]
+
+        if modem_path != '/sim900_0':


Do we have to have the sim900 modem as the default modem? As in, what's wrong if it's not?

It's a check because if you run ofono with some plugins, you might have a /stk_test virtual modem (standing for SimToolKit test). I wanted to be able to check the modem was correctly initialized at this point, but it might be an issue when we switch to SIM52XX later on. Shall I remove it ?

"check if it's correctly initialized" - you mean, check if it's available? If so, I'm guessing it might still be there in some cases, just not the 0th element in the list of the modems that you'll get. I suggest using if "/sim900_0" in manager.getModems()[0] or something along those lines.

CRImier · 2018-01-13T16:59:08Z

ofono/bridge.py

+    @property
+    def _bus(self):
+        # lil hack so we always have an up-to-date bus
+        return pydbus.SystemBus().get('org.ofono', '/sim900_0')


That "/sim900_0" string looks like it could benefit from being an attribute, it's repeated 3 times in this class

CRImier · 2018-01-13T16:59:59Z

ofono/bridge.py

+        return self._get_dbus_interface('MessageManager')
+
+    def _get_dbus_interface(self, name):
+        ''.startswith('org.ofono')


What does this particular line of code do here? It seems like a leftover line of code.

CRImier · 2018-01-13T17:13:30Z

ofono/bridge.py

+def main():
+    ofono = OfonoBridge()
+    ofono.start()
+    mainloop = GLib.MainLoop()  # todo : own thread


I don't get it - you have the GLib.MainLoop, and you have the DBusGMainLoop initialized in the beginning - are you sure you need both?

CRImier · 2018-01-13T17:16:38Z

ofono/bridge.py

+
+    @property
+    def _bus(self):
+        # lil hack so we always have an up-to-date bus


What's happening with the bus? Why can it get out-of-date?

When you get the bus, you actually get a snapshot of the currently available methods.

So if you get the bus before the modem is initialized you don't have any method. Using it that way ensures you always have the latest exposed methods.

Understood! Would be great to put this explanation in the docstring/comment inside that function =)

CRImier · 2018-01-13T17:18:00Z

ofono/bridge.py

+        if modem_path != '/sim900_0':
+            raise ValueError("Default modem should be '/sim900_0', was '{}'".format(modem_path))
+
+        # self.start()  #todo: check if it's the suitable place to do it


I don't think so - it makes sense not to start() the loop right when the OfonoBridge is initialized.

As in, explicit is better than implicit =)

CRImier · 2018-01-13T17:23:20Z

ofono/bridge.py

+            try:
+                self._bus.SetProperty("Powered", pydbus.Variant('b', True))
+                sleep(2)  # Let the modem some time to initialize
+            except Exception:


Just except: is enough if you want a catch-all

I think that, if the modem can't power on, we'll want some more information - as much as the exception can provide us, and maybe more, so don't skimp on logging here =)

CRImier · 2018-01-13T17:31:39Z

ofono/bridge.py

+        super(ConversationManager, self).__init__()
+        self.folder = os.path.expanduser("~/.phone/sms/")  # todo: store as a constant somewhere
+        if not os.path.exists(self.folder):
+            os.mkdir(self.folder)


You'll probably want os.mkdirs - it works like mkdir -p

CRImier · 2018-01-13T17:35:17Z

Why do we need it as a separate package (in the ZPUI folder root), instead of having it as a library in one of the apps?

monsieurh · 2018-01-13T17:42:23Z

We don't need it as a separate package, but in my mind 'apps' are programs the user interact with. Ofono bridge is indeed a library on which will depends 'phone app', 'sms app' and maybe others. So i didn't know where to place it in the hierarchy. Open to suggestion !

CRImier · 2018-01-13T18:21:08Z

ofono/bridge.py

+        mainloop.run()
+    except KeyboardInterrupt:
+        logger.info("Caught CTRL-C:exiting without powering off...")
+    except AttributeError:


logger.exception instead of just error, and make it a catch-all (as in, except: instead of catching AttributeError)? Would be great to have exception information in the logs, as well as protect from future errors.

CRImier · 2018-06-16T17:52:06Z

I'm working on this now. First thing first, modem communications (and easy-to-use timestamps) can be seen by using this ofonod CLI:

OFONO_AT_DEBUG=1 ofonod -n -d 'plugins/sim900.c,src/*,drivers/atmodem/*' 2>&1 | ts -s

You can get the ts utility from the moreutils package. So:

the modem is being set to CFUN=4 (essentially, flight mode) on power_off(). On next ofono bootup, the modem doesn't respond to ATE0 (AT command echo off), even though the flight mode shouldn't turn the serial communications off. Resetting the modem and restarting ofonod fixes the problem - funnily enough, resetting the modem without restarting ofonod doesn't.
ofonod has good debugging abilities (though I do still want to add a double-RX-UART to my debugging setup)
I don't have to reboot the phone - all I need is this commandline, then restart ofonod and bridge.py:

python -c 'import zerophone_hw; zerophone_hw.GSM_Modem().reset()'

CRImier · 2018-06-16T20:05:18Z

Ofono is already run at startup (debian ships a .service file), shall we have our own fork, we'll use that

CRImier · 2018-06-17T03:25:31Z

I've copied the sim900 driver to a sim800 driver and made some changes, now I can add SIM800-specific stuff. First things first, I've changed CFUN=4 command to CFUN=1 (the state the modem already is in by default). However, switching still timeouts. Furthermore, serial communication is still broken after stopping ofono! Will research further.

…KeysInput to use baseViewMixin

codecov · 2019-01-06T06:18:23Z

Codecov Report

Merging #83 into devel will increase coverage by 0.14%.
The diff coverage is 55.6%.

@@            Coverage Diff             @@
##            devel      #83      +/-   ##
==========================================
+ Coverage   41.26%   41.41%   +0.14%     
==========================================
  Files         260      266       +6     
  Lines       21830    22224     +394     
==========================================
+ Hits         9008     9203     +195     
- Misses      12822    13021     +199

Impacted Files	Coverage Δ
ui/tests/test_canvas.py	`84.61% <ø> (-0.93%)`	⬇️
libs/ofono/bridge.py	`0% <0%> (ø)`
apps/phone/graphics.py	`100% <100%> (ø)`
ui/dialog.py	`94.33% <100%> (+0.76%)`	⬆️
ui/__init__.py	`100% <100%> (ø)`	⬆️
ui/overlays.py	`79.81% <100%> (-3.6%)`	⬇️
ui/numpad_input.py	`77.44% <100%> (+0.49%)`	⬆️
ui/tests/test_numpad_input.py	`86.99% <100%> (+2.08%)`	⬆️
ui/base_list_ui.py	`84.61% <100%> (+2.14%)`	⬆️
apps/phone/views.py	`26.78% <26.78%> (ø)`
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b7cf8c4...5af4c3c. Read the comment docs.

CRImier · 2019-01-06T06:24:46Z

ouch, accident

…ocess the mapping in view's __init__

…tegerAdjust to use views

…oving cflip out of canvas.py, exposting all these functions (and more) in ui/__init__.py

…um_keys

…ld've been done initially), changing the action_key leakage test to mapping leakage test

…ntext name

monsieurh added 4 commits January 7, 2018 20:05

adds a basic OfonoBridge class to interact with Ofono through D-Bus

8b385ae

adds `import_sim_phonebook` to `OfonoBridge`

#50 restart ofono-bridge development

e1a63fb

#50 adds udev rule for SIM800L

f43ea67

#50 WIP ofono-bridge

1994095

monsieurh mentioned this pull request Jan 13, 2018

[Discussion] ofono bridge #50

Open

monsieurh changed the title ~~Ofono bridge~~ [WIP] Ofono bridge Jan 13, 2018

monsieurh added help wanted needs testing labels Jan 13, 2018

monsieurh added 2 commits January 13, 2018 17:42

Merge remote-tracking branch 'origin/devel' into ofono_bridge

0413ddf

fixes duplicate entry in requirements.txt

ff3d603

CRImier reviewed Jan 13, 2018

View reviewed changes

adds some safeguards

b5d29b0

CRImier reviewed Jan 13, 2018

View reviewed changes

monsieurh and others added 12 commits January 15, 2018 13:58

fixes logger import

67c2baa

refactors '/sim900_0' as a member variable

0966d0e

refactors the modem check as a method _check_default_modem

12d27f9

updates self._bus hack explanation

41ff44c

remove development leftover

e1f101b

better exception handling

f085e7b

replaces print by logger.info

2a084a6

now raises an exception when an interface is not found

08dc9e8

ConversationManager now recursively create log folder

da2fb47

Merge branch 'devel' into ofono_bridge

1650127

Adding a default log level - fixing tests

0cf96f2

Movong the ofono libraries into a separate library

8e50fd7

CRImier added 8 commits December 19, 2018 04:21

Merge branch 'devel' into ofono_bridge

248a30a

Thinking about architecture - initial changes

54d548f

Merge branch 'devel' into ofono_bridge

49a1ee1

Adding the BaseViewMixin, porting UI elements to use it

becaef5

Fixing method's return type in declaration

85e74ce

Number input, overlays and value pagination work

2a40a20

Adding BaseView class and porting everything to it, porting CharArrow…

47350dc

…KeysInput to use baseViewMixin

Ported InputScreenView to use BaseView

917f7f6

Forgot a small thing

44d6f44

CRImier closed this Jan 6, 2019

CRImier reopened this Jan 6, 2019

CRImier added 15 commits January 6, 2019 13:05

Broke the launch process, fixing

b04f185

moving BaseViewMixin.__init__ - making it possible for the view to pr…

9887af1

…ocess the mapping in view's __init__

More testing and optimizations in the dialer

85cf72c

Adding BaseView functions for the usual character refresh, porting In…

377c146

…tegerAdjust to use views

Moving overlays to use update_keymap

1d29e7f

Updating the overlay options for better button usage

f92f021

Adding a bunch of functions to work with coordinates, adding tests, m…

378593e

…oving cflip out of canvas.py, exposting all these functions (and more) in ui/__init__.py

Calculate FunctionOverlay labels depending on len of labels and not n…

c787dc3

…um_keys

Numpad*Input now backgroundable

ec17696

More phone app work - status screen ready

886a243

Forgot to remove outdated test, adding

801fb09

Making the Numpad*Input use the keymap for action buttons (as it shou…

3a3500b

…ld've been done initially), changing the action_key leakage test to mapping leakage test

Accidentally changed context names, fixing the hardcoded exclusive co…

65e450e

…ntext name

Adding "start context" function - for custom apps

bb9e456

Merge branch 'devel' into ofono_bridge

5af4c3c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Ofono bridge #83

[WIP] Ofono bridge #83

monsieurh commented Jan 13, 2018 •

edited by CRImier

Loading

CRImier Jan 13, 2018 •

edited

Loading

CRImier Jan 13, 2018

CRImier Jan 13, 2018

CRImier Jan 13, 2018

CRImier Jan 13, 2018

CRImier Jan 13, 2018

monsieurh Jan 15, 2018

CRImier Jan 15, 2018

CRImier Jan 13, 2018

monsieurh Jan 15, 2018

CRImier Jan 13, 2018

CRImier Jan 13, 2018

CRImier Jan 13, 2018

monsieurh Jan 15, 2018

CRImier Jan 15, 2018

CRImier Jan 13, 2018

CRImier Jan 13, 2018

CRImier Jan 13, 2018

monsieurh Jan 15, 2018

CRImier Jan 13, 2018

CRImier commented Jan 13, 2018

monsieurh commented Jan 13, 2018

CRImier Jan 13, 2018

CRImier commented Jun 16, 2018 •

edited

Loading

CRImier commented Jun 16, 2018

CRImier commented Jun 17, 2018

codecov bot commented Jan 6, 2019 •

edited

Loading

CRImier commented Jan 6, 2019


		from helpers import Singleton, setup_logger

		DBusGMainLoop(set_as_default=True) # has to be called first

		@@ -0,0 +1,2 @@
		KERNEL=="ttyAMA0", ENV{OFONO_DRIVER}="sim900"

[WIP] Ofono bridge #83

Are you sure you want to change the base?

[WIP] Ofono bridge #83

Conversation

monsieurh commented Jan 13, 2018 • edited by CRImier Loading

ofono bridge progress

purpose

implementation

issues

todo

zpui

ofono

system

CRImier Jan 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CRImier commented Jan 13, 2018

monsieurh commented Jan 13, 2018

Choose a reason for hiding this comment

CRImier commented Jun 16, 2018 • edited Loading

CRImier commented Jun 16, 2018

CRImier commented Jun 17, 2018

codecov bot commented Jan 6, 2019 • edited Loading

Codecov Report

CRImier commented Jan 6, 2019

monsieurh commented Jan 13, 2018 •

edited by CRImier

Loading

CRImier Jan 13, 2018 •

edited

Loading

CRImier commented Jun 16, 2018 •

edited

Loading

codecov bot commented Jan 6, 2019 •

edited

Loading