[Merge] lp:~sergiusens/nuntium/deactivate into lp:nuntium
Sergio Schvezov
sergio.schvezov at canonical.com
Wed Oct 1 22:55:55 UTC 2014
address the comments inline, will proceed to fixing now
Diff comments:
> === modified file 'mediator.go'
> --- mediator.go 2014-10-01 14:51:51 +0000
> +++ mediator.go 2014-10-01 14:51:51 +0000
> @@ -25,6 +25,7 @@
> "io/ioutil"
> "log"
> "os"
> + "sync"
>
> "launchpad.net/nuntium/mms"
> "launchpad.net/nuntium/ofono"
> @@ -44,6 +45,7 @@
> NewMSendReqFile chan struct{ filePath, uuid string }
> outMessage chan *telepathy.OutgoingMessage
> terminate chan bool
> + contextLock sync.Mutex
> }
>
> //TODO these vars need a configuration location managed by system settings or
> @@ -164,6 +166,9 @@
> }
>
> func (mediator *Mediator) getMRetrieveConf(mNotificationInd *mms.MNotificationInd) {
> + mediator.contextLock.Lock()
> + defer mediator.contextLock.Unlock()
> +
> preferredContext, _ := mediator.telepathyService.GetPreferredContext()
> mmsContext, err := mediator.modem.ActivateMMSContext(preferredContext)
> if err != nil {
> @@ -185,6 +190,9 @@
> } else {
> storage.UpdateDownloaded(mNotificationInd.UUID, filePath)
> }
> + if err := mediator.modem.DeactivateMMSContext(mmsContext); err != nil {
Good catch, will fix
> + log.Println("Issues while deactivating context:", err)
> + }
> mediator.NewMRetrieveConfFile <- mNotificationInd.UUID
> }
>
> @@ -360,6 +368,9 @@
> }
>
> func (mediator *Mediator) uploadFile(filePath string) (string, error) {
> + mediator.contextLock.Lock()
> + defer mediator.contextLock.Unlock()
> +
> preferredContext, _ := mediator.telepathyService.GetPreferredContext()
> mmsContext, err := mediator.modem.ActivateMMSContext(preferredContext)
> if err != nil {
> @@ -376,5 +387,11 @@
> if err != nil {
> return "", err
> }
> - return mms.Upload(filePath, msc, proxy.Host, int32(proxy.Port))
> + mSendRespFile, uploadErr := mms.Upload(filePath, msc, proxy.Host, int32(proxy.Port))
> +
> + if err := mediator.modem.DeactivateMMSContext(mmsContext); err != nil {
will fix deactivation call.
there is no hard requirement, GetProxy is a noop if not set, but udm still requrires it (empty string, port 0)
> + log.Println("Issues while deactivating context:", err)
> + }
> +
> + return mSendRespFile, uploadErr
> }
>
> === modified file 'ofono/modem.go'
> --- ofono/modem.go 2014-10-01 14:51:51 +0000
> +++ ofono/modem.go 2014-10-01 14:51:51 +0000
> @@ -29,6 +29,7 @@
> "reflect"
> "strconv"
> "strings"
> + "time"
>
> "launchpad.net/go-dbus/v1"
> )
> @@ -38,6 +39,10 @@
> contextTypeMMS = "mms"
> )
>
> +const (
> + ofonoInProgressError = "org.ofono.InProgress"
> +)
> +
> type OfonoContext struct {
> ObjectPath dbus.ObjectPath
> Properties PropertiesType
> @@ -245,30 +250,96 @@
> }
> log.Println("Trying to activate context on", context.ObjectPath)
> obj := modem.conn.Object("org.ofono", context.ObjectPath)
> - _, err = obj.Call(CONNECTION_CONTEXT_INTERFACE, "SetProperty", "Active", dbus.Variant{true})
> - if err != nil {
> - log.Printf("Cannot Activate interface on %s: %s", context.ObjectPath, err)
> - } else {
> - return context, nil
> + for i := 0; i < 3; i++ {
> + r, err := obj.Call(CONNECTION_CONTEXT_INTERFACE, "SetProperty", "Active", dbus.Variant{true})
> + if err == nil {
> + return context, nil
> + } else if err != nil && r.ErrorName == ofonoInProgressError {
> + log.Printf("Cannot Activate (try %d/3) interface on %s: %s", i+1, context.ObjectPath, err)
> + time.Sleep(2 * time.Second)
> + }
> + log.Println("Unhandled dbus error", r.ErrorName, "while activating context ...skipping wait")
I'll add .AttachInProgress, but already added .NotAttached in the MP that follows this one
> }
> }
> return OfonoContext{}, errors.New("no context available to activate")
> }
>
> +//DeactivateMMSContext deactivates the context if it is of type mms
> +func (modem *Modem) DeactivateMMSContext(context OfonoContext) error {
> + if context.isTypeInternet() {
> + return nil
> + }
> +
> + log.Println("Trying to deactivate context on", context.ObjectPath)
> + obj := modem.conn.Object("org.ofono", context.ObjectPath)
> + var err error
> + var r *dbus.Message
> + for i := 0; i < 3; i++ {
> + r, err = obj.Call(CONNECTION_CONTEXT_INTERFACE, "SetProperty", "Active", dbus.Variant{false})
> + if err == nil {
> + return nil
> + } else if err != nil && r.ErrorName == ofonoInProgressError {
> + log.Printf("Cannot Deactivate (try %d/3) interface on %s: %s", i+1, context.ObjectPath, err)
> + time.Sleep(2 * time.Second)
> + }
> + log.Println("Unhandled dbus error", r.ErrorName, "while deactivating context ...skipping wait")
I did some testing and commented on IRC, .NotAttached does not affect deactivation. I'll check for .AttachInProgress
I used the ofono scripts btw:
activate-context #
list-contexts (to verify it was active)
disable-gprs
list-contexts (it still listed the context as active, not sure it is right or wrong as we are no longer attached according to list-modems)
deactivate-context (works like a charm).
> + }
> + return err
> +}
> +
> +func (oContext OfonoContext) isTypeInternet() bool {
> + if v, ok := oContext.Properties["Type"]; ok {
> + return reflect.ValueOf(v.Value).String() == contextTypeInternet
> + }
> + return false
> +}
> +
> +func (oContext OfonoContext) isTypeMMS() bool {
> + if v, ok := oContext.Properties["Type"]; ok {
> + return reflect.ValueOf(v.Value).String() == contextTypeMMS
> + }
> + return false
> +}
> +
> func (oContext OfonoContext) isActive() bool {
> return reflect.ValueOf(oContext.Properties["Active"].Value).Bool()
> }
>
> +func (oContext OfonoContext) hasMessageCenter() bool {
> + return oContext.messageCenter() != ""
> +}
> +
> +func (oContext OfonoContext) messageCenter() string {
> + if v, ok := oContext.Properties["MessageCenter"]; ok {
> + return reflect.ValueOf(v.Value).String()
> + }
> + return ""
> +}
> +
> +func (oContext OfonoContext) messageProxy() string {
> + if v, ok := oContext.Properties["MessageProxy"]; ok {
> + return reflect.ValueOf(v.Value).String()
> + }
> + return ""
> +}
> +
> +func (oContext OfonoContext) name() string {
> + if v, ok := oContext.Properties["Name"]; ok {
> + return reflect.ValueOf(v.Value).String()
> + }
> + return ""
> +}
> +
> func (oContext OfonoContext) GetMessageCenter() (string, error) {
> - if msc := reflect.ValueOf(oContext.Properties["MessageCenter"].Value).String(); msc != "" {
> - return msc, nil
> + if oContext.hasMessageCenter() {
> + return oContext.messageCenter(), nil
> } else {
> return "", errors.New("context setting for the Message Center value is empty")
> }
> }
>
> func (oContext OfonoContext) GetProxy() (proxyInfo ProxyInfo, err error) {
> - proxy := reflect.ValueOf(oContext.Properties["MessageProxy"].Value).String()
> + proxy := oContext.messageProxy()
> // we need to support empty proxies
> if proxy == "" {
> return proxyInfo, nil
> @@ -309,33 +380,8 @@
> }
>
> for _, context := range contexts {
> - var name, contextType, msgCenter, msgProxy string
> - var active bool
> - for k, v := range context.Properties {
> - if reflect.ValueOf(k).Kind() != reflect.String {
> - continue
> - }
> - k = reflect.ValueOf(k).String()
> - switch k {
> - case "Name":
> - name = reflect.ValueOf(v.Value).String()
> - case "Type":
> - contextType = reflect.ValueOf(v.Value).String()
> - case "MessageCenter":
> - msgCenter = reflect.ValueOf(v.Value).String()
> - case "MessageProxy":
> - msgProxy = reflect.ValueOf(v.Value).String()
> - case "Active":
> - active = reflect.ValueOf(v.Value).Bool()
> - }
> - }
> - log.Println("Check context valid for - Name", name,
> - "| Context type:", contextType,
> - "| MessageCenter:", msgCenter,
> - "| MessageProxy:", msgProxy,
> - "| Active:", active)
> - if (contextType == contextTypeInternet && active && msgCenter != "") || contextType == contextTypeMMS {
> - if context.ObjectPath == preferredContext || active {
> + if (context.isTypeInternet() && context.isActive() && context.hasMessageCenter()) || context.isTypeMMS() {
> + if context.ObjectPath == preferredContext || context.isActive() {
> mmsContexts = append([]OfonoContext{context}, mmsContexts...)
> } else {
> mmsContexts = append(mmsContexts, context)
> @@ -343,6 +389,7 @@
> }
> }
> if len(mmsContexts) == 0 {
> + log.Printf("non matching contexts:\n %+v", contexts)
> return mmsContexts, errors.New("No mms contexts found")
> }
> return mmsContexts, nil
>
--
https://code.launchpad.net/~sergiusens/nuntium/deactivate/+merge/236724
Your team Ubuntu Phablet Team is subscribed to branch lp:nuntium.
More information about the Ubuntu-reviews
mailing list