openbsc[master]: Add twisted-based IPA multiplex

[Neels Hofmeyr]

Patch Set 10:

(7 comments)

I'm confused, what does this code actually *do*??

https://gerrit.osmocom.org/#/c/1268/10/openbsc/contrib/twisted_ipa.py
File openbsc/contrib/twisted_ipa.py:

Line 33:     Generic IPA protocol handler: include some routines for simpler subprotocols
end sentence with a '.', can't rely on line break in API docs


Line 46:         """
I'm not sure that this comment is helpful at all.
Also I would put it on a single line like

  def foo():
    'single line comment'
    code

same below


Line 48:             print('OSMO MGCP received %s' % data)
rather have one

  def dbg(*args):
    if not self.factory.debug:
      return
    print(' '.join(*args))

or similar, then only use self.dbg(msg) all over this file.


Line 92:     def handle_RSL(self, d, pr, ex):
what are d, pr, ex? let's not have these cryptic names without a doc


Line 97:             print('IPA RSL received message with attribute %s' % ex)
hmm, here 'ex' is called 'attribute'? looks like a naming choice problem. same below.


Line 104:         pass
hmm really? if i pick the wrong class, CCM messages are blown in the wind without anyone noticing? Wait a minute, none of these functions actually *do* anything? Could this API be designed better in the first place?


Line 332:         Only necessayr for as debugging aid - if we can somehow set parent's class noisy attribute than we can omit this method
"necessary", same typo seen around elswhere.
Also it's "then" when talking about time -- "than" is for comparison


-- 
To view, visit https://gerrit.osmocom.org/1268
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I07559df420b7fe8418f3412f45acd9a375e43bc5
Gerrit-PatchSet: 10
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-HasComments: Yes