Skip to content

Conversation

@alfonsosanchezbeato
Copy link
Contributor

Add support for snappy by checking env vars to know where to store/find data files.

Copy link
Contributor

@tonyespy tonyespy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per our discussion the other day, let's merge and/or push this into a separate branch for now.

Also didn't see any changes for the ubuntu-apndn plugin?

char *filename;

snap = getenv("SNAP");
filename = g_strdup_printf("%s%s", snap ? snap : "", MBPI_DATABASE);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should discuss where mbpi ends up living as it's used by modemmanager too.

I think it probably should live in it's own snap that shares the file via content sharing. We also should explore the ability for an end-user to modify this file on a working system ( ie. allow a writable version that would override the default ) vs. forcing the user to report a bug and wait for an official update.

gboolean ret = FALSE;
const char *snap;
char *filename;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a short comment that explains the SNAP env variable.

const char *filename;
char *snap_filename = NULL;
const char *conf_override = getenv("OFONO_PHONESIM_CONFIG");
const char *snap_common = getenv("SNAP_COMMON");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a short comment explaining why SNAP_COMMON is used here.


g_key_file_free(keyfile);
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a short comment here describing the purpose of this function, and why SNAP_COMMON is used.

@tonyespy
Copy link
Contributor

Might as well add some extra logic to plugins/u8500.c, as it won't be possible for a user to add the file /etc/imei.

@alfonsosanchezbeato
Copy link
Contributor Author

Branch refreshed after addressing comments. Some points:

  1. Yes, we agreed on creating a new snappy branch, and also on following the discussion here until the MP is given thumbs up. Let's do that, and create a new MP after that.
  2. Take into account that apns-conf.xml is part of the Android container. Before changing ubuntu-apndb we have to add it to ofono. In fact I remember to have discussed about this before, and the conclusion was that we should move the DB to the ofono package. We can discuss this online.
  3. Having MBPI as a snap would introduce dependencies, which would go against snappy design.
  4. No, MBPI should not be writable by the user, as apns-conf.xml is not either in Touch. If the operator is not present, the user will have to change the provisioning data, which will be saved by ofono anyway, so this needs to be done only once. Being able to modify the DB will not make this less painful for the user. Of course, updating the DB for these operators is great, but we do not need to let the user modify (which might led to corruption) the local DB for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants