(dormant account) Reporter
	Description • 13 years ago

A number of people asked that we turn off telemetry when on 3G. Currently there is no api to identify connection type.

	Doug Turner (:dougt)
	Comment 1 • 13 years ago

as for types, the DAP is currently uses these strings:

    "unknown" if the type of this connection is unknown.
    "ethernet" if this is an Ethernet connection.
    "usb" if this is a connection over a USB port.
    "wifi" if this is a Wi-Fi connection (i.e. IEEE802.11).
    "wimax" if this is a WiMax connection (i.e. IEEE802.16).
    "2g" if this is a 2G connection (e.g. GSM, GPRS, EDGE).
    "3g" if this is a 3G connection (e.g. UMTS, CDMA).
    "4g" if this is a 4G connection (e.g. LTE, UMB).


Also, it is fair to note that this doesn't guarantee throughput.  (For example, at my office, the WiFI sometimes is slower than my 3g network).

	Doug Turner (:dougt)
	Comment 2 • 13 years ago

i am thinking this should just be a simple event:

window.addEventListener("network-connection", function(event) {
  if (event.type == "wifi")
   dosomething();
}

	(dormant account) Reporter
	Comment 3 • 13 years ago

A listener is good t have, but for my particular case it would be more convenient to be able to call a method somewhere. Either way works for me.

	Doug Turner (:dougt)
	Comment 4 • 13 years ago

okay.

	Doug Turner (:dougt)
	Comment 5 • 13 years ago

the first part is to get the gecko system to report the network connection in a consistent manner.  I think we should put the current connection type in the SystemInfo property bag, and broadcast when it changes using the nsIObserverService.  (Taras, that would probably work for you).

The next step would be to make this a more DOMie API.  That can be a follow up.

	(dormant account) Reporter
	Comment 7 • 13 years ago

(In reply to comment #5)
> the first part is to get the gecko system to report the network connection
> in a consistent manner.  I think we should put the current connection type
> in the SystemInfo property bag, and broadcast when it changes using the
> nsIObserverService.  (Taras, that would probably work for you).

sounds good

	Doug Turner (:dougt)
	Comment 9 • 13 years ago

Comment on attachment 542575 [details] [diff] [review]
android hack

needs, at least:

* a mapping from the android subtype to the names we know about
* storing the current value in the system info property bag.

	Chris Lord [:cwiiis]
	Comment 10 • 13 years ago

Patch based on Doug's android hack - adds an attribute to nsINetworkLinkService that lets you query the network type, and notifies the network type when it changes.

Does this provide what you needed?

Things to think about:

- What types of network do we want to expose? Do we want a coarse set of definitions (as in this patch), or something a lot more fine-grained?
- Expose via some kind of DOM api?

	Chris Lord [:cwiiis]
	Comment 11 • 13 years ago

Whoops, missed the bit about system-info - taking a look at that now.

	Chris Lord [:cwiiis]
	Comment 12 • 13 years ago

Looking at system info, does this information really belong there? Seems to only contain static information about the system (cpu + memory info, hardware types, etc.)

I missed comment #2, will update the patch and use consistent names.

	Doug Turner (:dougt)
	Comment 13 • 13 years ago

you could hang the state information off of nsINetworkLinkService

	Chris Lord [:cwiiis]
	Comment 14 • 13 years ago

Just a note, I have what is hopefully a better version of this patch in the pipeline, but it's crashing at the moment (so I guess it isn't better yet..) - will upload tomorrow once I've sorted that out.

Also, the patch does make the state information available via nsINetworkLinkService. (the current one does this too)

	Chris Lord [:cwiiis]
	Comment 15 • 13 years ago

This patch goes a bit further than the initial patch - it adds the same property to nsINetworkLinkService and does the same notification, but it uses the categories listed in comment #1, and has code to determine what type of mobile connection is being used.

This unfortunately requires an extra permission (read-phone-state).

	Chris Lord [:cwiiis]
	Comment 16 • 13 years ago

This patch refreshes the uuid in the idl file and includes stub implementations for all the platforms.

Only Android is actually implemented, all other platforms return 'unknown' for the connection type.

An implementation on the NetworkManager NetworkLinkStatus object would be completely do-able, but quite tedious. You'd need to read the NM_DEVICE_TYPE property of the org.freedesktop.NetworkManager.Device object (which you'd have to retrieve asynchronously on state-change).

Much like on Android where you have to read the capabilities from the TelephonyManager to know what kind of 'mobile' connection it is, likewise you'd need to read the NM_DEVICE_MODEM_CAPABILITIES property of the org.freedesktop.NetworkManager.Device.Modem object to figure out what kind of mobile connection it is in NetworkManager (thankfully this list of capabilities maps much more easily to 2g/3g/4g than Android's multitudinous collection of types).

This is pretty tedious because you ought not to do synchronous calls on dbus, and we're using dbus directly rather than a convenience library to interface with NetworkManager.

	Doug Turner (:dougt)
	Comment 17 • 13 years ago

Comment on attachment 543302 [details] [diff] [review]
Patch that includes stub implementations for all platforms

Review of attachment 543302 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good.  one more patch, and I think we are done.

::: embedding/android/GeckoApp.java
@@ +357,5 @@
> +            GeckoApp.mAppContext.getSystemService(Context.TELEPHONY_SERVICE);
> +        tm.listen(mPhoneStateListener, PhoneStateListener.LISTEN_DATA_CONNECTION_STATE);
> +
> +        // Notify if network state changed since we paused
> +        GeckoAppShell.onNetworkStateChange(true);

why do we have to add this onNetworkStatusChanged() call?

::: embedding/android/GeckoAppShell.java
@@ +94,5 @@
>      static private File sCacheFile = null;
>      static private int sFreeSpace = -1;
>  
> +    static private String sNetworkState;
> +    static private String sNetworkType;

Initialize these.

@@ +658,5 @@
>          GeckoApp.setLaunchState(GeckoApp.LaunchState.GeckoRunning);
>          sendPendingEventsToGecko();
> +
> +        // Refresh the network connectivity state
> +        onNetworkStateChange(false);

Why is this needed?

@@ +1017,5 @@
>  
>      public static boolean isNetworkLinkUp() {
> +        if (sNetworkState == "up")
> +            return true;
> +        else

non-sequitur else

just do:

if (whatever)
  return true
return false

@@ +1024,5 @@
> +
> +    public static boolean isNetworkLinkKnown() {
> +        if (sNetworkState == "unknown")
> +            return false;
> +        else

same

::: netwerk/base/public/nsINetworkLinkService.idl
@@ +65,5 @@
> +
> +  /**
> +   * The type of network connection.
> +   */
> +  readonly attribute string linkType;

although this is perfectly fine the way it is -- i wonder if script developers would like constants they can use?

::: netwerk/system/android/nsAndroidNetworkLinkService.cpp
@@ +74,5 @@
> +NS_IMETHODIMP
> +nsAndroidNetworkLinkService::GetLinkType(char **aLinkType)
> +{
> +  NS_ENSURE_ARG_POINTER(aLinkType);
> +  NS_ENSURE_TRUE(mozilla::AndroidBridge::Bridge(), NS_ERROR_NOT_IMPLEMENTED);

Wrong error.  if the bridge is not found, we are hosed.  NS_ERROR_UNEXPECTED?

@@ +81,5 @@
> +  if (mozilla::AndroidBridge::Bridge()->GetNetworkLinkType(linkType)) {
> +      *aLinkType = ToNewUTF8String(linkType);
> +      if (!(*aLinkType))
> +          return NS_ERROR_OUT_OF_MEMORY;
> +      else

drop the else.

maybe the logic should be:

if (*aLinkType)
  return NS_OK;

then have everything fall out to the last return

::: netwerk/system/maemo/nsMaemoNetworkLinkService.cpp
@@ +75,5 @@
> +nsMaemoNetworkLinkService::GetLinkType(char **aLinkType)
> +{
> +  NS_ENSURE_ARG_POINTER(aLinkType);
> +
> +  *aLinkType = NS_strdup(NS_NETWORK_LINK_TYPE_UNKNOWN);

add a comment like "stub implementation, on this platform we don't know what to do yet".

::: widget/src/android/AndroidBridge.cpp
@@ +681,5 @@
> +                                                             jGetNetworkLinkType));
> +    if (!jstrType)
> +        return PR_FALSE;
> +
> +    nsJNIString jniStr(jstrType);

jniStr(jstrType, mJNIEnv)

	Chris Lord [:cwiiis]
	Comment 18 • 13 years ago

(In reply to comment #17)
> Comment on attachment 543302 [details] [diff] [review] [review]
> Patch that includes stub implementations for all platforms
> 
> Review of attachment 543302 [details] [diff] [review] [review]:
> -----------------------------------------------------------------
> 
> Looks good.  one more patch, and I think we are done.
> 
> ::: embedding/android/GeckoApp.java
> @@ +357,5 @@
> > +            GeckoApp.mAppContext.getSystemService(Context.TELEPHONY_SERVICE);
> > +        tm.listen(mPhoneStateListener, PhoneStateListener.LISTEN_DATA_CONNECTION_STATE);
> > +
> > +        // Notify if network state changed since we paused
> > +        GeckoAppShell.onNetworkStateChange(true);
> 
> why do we have to add this onNetworkStatusChanged() call?

When the app is paused, we stop listening to network status changes. This is in the situation that the network status changes while we're paused - it won't notify if it hasn't changed.

> ::: embedding/android/GeckoAppShell.java
> @@ +94,5 @@
> >      static private File sCacheFile = null;
> >      static private int sFreeSpace = -1;
> >  
> > +    static private String sNetworkState;
> > +    static private String sNetworkType;
> 
> Initialize these.

Ok

> @@ +658,5 @@
> >          GeckoApp.setLaunchState(GeckoApp.LaunchState.GeckoRunning);
> >          sendPendingEventsToGecko();
> > +
> > +        // Refresh the network connectivity state
> > +        onNetworkStateChange(false);
> 
> Why is this needed?

So we have the correct values in sNetworkState and sNetworkType on startup. We always return the cached values, so we need to update them on startup.

> @@ +1017,5 @@
> >  
> >      public static boolean isNetworkLinkUp() {
> > +        if (sNetworkState == "up")
> > +            return true;
> > +        else
> 
> non-sequitur else
> 
> just do:
> 
> if (whatever)
>   return true
> return false

Ok

> @@ +1024,5 @@
> > +
> > +    public static boolean isNetworkLinkKnown() {
> > +        if (sNetworkState == "unknown")
> > +            return false;
> > +        else
> 
> same

Ok

> ::: netwerk/base/public/nsINetworkLinkService.idl
> @@ +65,5 @@
> > +
> > +  /**
> > +   * The type of network connection.
> > +   */
> > +  readonly attribute string linkType;
> 
> although this is perfectly fine the way it is -- i wonder if script
> developers would like constants they can use?

Yes - shall I use string constants or numbers? If numbers, how do you normally go about passing numbers in aData of notifications?

> ::: netwerk/system/android/nsAndroidNetworkLinkService.cpp
> @@ +74,5 @@
> > +NS_IMETHODIMP
> > +nsAndroidNetworkLinkService::GetLinkType(char **aLinkType)
> > +{
> > +  NS_ENSURE_ARG_POINTER(aLinkType);
> > +  NS_ENSURE_TRUE(mozilla::AndroidBridge::Bridge(), NS_ERROR_NOT_IMPLEMENTED);
> 
> Wrong error.  if the bridge is not found, we are hosed.  NS_ERROR_UNEXPECTED?

This is what happens elsewhere in the file - I agree, I'd have thought NS_ERROR_FAILED or UNEXPECTED would make more sense... Should I change this for the other methods?

> @@ +81,5 @@
> > +  if (mozilla::AndroidBridge::Bridge()->GetNetworkLinkType(linkType)) {
> > +      *aLinkType = ToNewUTF8String(linkType);
> > +      if (!(*aLinkType))
> > +          return NS_ERROR_OUT_OF_MEMORY;
> > +      else
> 
> drop the else.
> 
> maybe the logic should be:
> 
> if (*aLinkType)
>   return NS_OK;
> 
> then have everything fall out to the last return

Ok, will switch around the logic to get rid of else's

> ::: netwerk/system/maemo/nsMaemoNetworkLinkService.cpp
> @@ +75,5 @@
> > +nsMaemoNetworkLinkService::GetLinkType(char **aLinkType)
> > +{
> > +  NS_ENSURE_ARG_POINTER(aLinkType);
> > +
> > +  *aLinkType = NS_strdup(NS_NETWORK_LINK_TYPE_UNKNOWN);
> 
> add a comment like "stub implementation, on this platform we don't know what
> to do yet".

Ok

> ::: widget/src/android/AndroidBridge.cpp
> @@ +681,5 @@
> > +                                                             jGetNetworkLinkType));
> > +    if (!jstrType)
> > +        return PR_FALSE;
> > +
> > +    nsJNIString jniStr(jstrType);
> 
> jniStr(jstrType, mJNIEnv)

Again, this was the same as several other places in the file - Is there a reason to use this different structure?

	Chris Lord [:cwiiis]
	Comment 19 • 13 years ago

Of course, the attribute on the interface doesn't need to correspond with the signal, so perhaps ignore my comment about aData and I'll assume we want number constants, as is customary in other idl's

	Chris Lord [:cwiiis]
	Comment 20 • 13 years ago

Hopefully this addresses all the concerns above. As GetLinkType now returns an int, that removes the issue with the string and I changed the return type from NOT_IMPLEMENTED to UNEXPECTED (we can change the others some other time).

Seems to build/work on android and Linux, need to ping IT about my account to push to try...

	Doug Turner (:dougt)
	Comment 21 • 13 years ago

Comment on attachment 543985 [details] [diff] [review]
Revised patch based on review

http://tbpl.mozilla.org/?tree=Try&rev=dae2ceb818c0

	Chris Lord [:cwiiis]
	Comment 22 • 13 years ago

Darn, this doesn't build because some of the network types were added in later versions of the API - Details;

NETWORK_TYPE_EVDO_B was added in API level 9,
NETWORK_TYPE_EHRPD was added in API level 11,
NETWORK_TYPE_LTE was added in API level 11.

Seeing as these are just constants, I'll replace them with numbers and a comment with explanation.

Surprised that LTE is only in API level 11, I wonder where that leaves current Gingerbread phones that support LTE?

	Chris Lord [:cwiiis]
	Comment 23 • 13 years ago

Same patch that just uses the numbers along with comments explaining why for the fields that only appear in API level > 8.

	Jason Duell
	Comment 24 • 13 years ago

Comment on attachment 544168 [details] [diff] [review]
Don't use fields from above API level 8

Review of attachment 544168 [details] [diff] [review]:
-----------------------------------------------------------------

Just a few drive-by comments:

> // XXX This function has not yet been implemented for this platform

We should file followup bugs to implement GetLinkType for our other platforms:  It's likely to come in handy for many things.  I'm not sure who the usual suspects for the various platforms are: if anyone does know and can cc them on the bug for their platform(s), that'd be awesome.  If you're feeling pressed for time, email me any names and I can open the bugs.  Thanks!

::: embedding/android/GeckoAppShell.java
@@ +1098,5 @@
> +        if (notifyChanged && (state != sNetworkState || typeCode != sNetworkTypeCode)) {
> +            Log.i("GeckoAppShell", "Network state changed: (" + state + ", " + type + ") ");
> +            sNetworkState = state;
> +            sNetworkType = type;
> +            sNetworkTypeCode = typeCode;

Just want to make sure it's intentional that you only update sNetworkState/Type/Code when you're doing notification.  I.e. that this function should notify if the network state has changed since the last onNetworkState(true) call, and intermediate onNetworkState(false) calls don't "count" as noticing a state change.

::: netwerk/base/public/nsINetworkLinkService.idl
@@ +102,5 @@
>  #define NS_NETWORK_LINK_DATA_UNKNOWN "unknown"
> +
> +/**
> + * We send notifications through nsIObserverService with topic
> + * NS_NETWORK_LINK_TYPE_TOPIC whenever the network conection type

s/conection/connection/

	Chris Lord [:cwiiis]
	Comment 25 • 13 years ago

(In reply to comment #24)
> Comment on attachment 544168 [details] [diff] [review] [review]
> Don't use fields from above API level 8
> 
> Review of attachment 544168 [details] [diff] [review] [review]:
> -----------------------------------------------------------------
> 
> Just a few drive-by comments:
> 
> > // XXX This function has not yet been implemented for this platform
> 
> We should file followup bugs to implement GetLinkType for our other
> platforms:  It's likely to come in handy for many things.  I'm not sure who
> the usual suspects for the various platforms are: if anyone does know and
> can cc them on the bug for their platform(s), that'd be awesome.  If you're
> feeling pressed for time, email me any names and I can open the bugs. 
> Thanks!

Yes, we'll do this - if you know which people to contact for which platforms, that'd be great, otherwise I'll just look at who's been editing the relevant files and go on that.

> ::: embedding/android/GeckoAppShell.java
> @@ +1098,5 @@
> > +        if (notifyChanged && (state != sNetworkState || typeCode != sNetworkTypeCode)) {
> > +            Log.i("GeckoAppShell", "Network state changed: (" + state + ", " + type + ") ");
> > +            sNetworkState = state;
> > +            sNetworkType = type;
> > +            sNetworkTypeCode = typeCode;
> 
> Just want to make sure it's intentional that you only update
> sNetworkState/Type/Code when you're doing notification.  I.e. that this
> function should notify if the network state has changed since the last
> onNetworkState(true) call, and intermediate onNetworkState(false) calls
> don't "count" as noticing a state change.

That's correct - I assume we don't want to send the changed notification on every startup (we didn't do so before, so maintaining existing behaviour), but we need to update the variables with the correct state in case they're read.

> ::: netwerk/base/public/nsINetworkLinkService.idl
> @@ +102,5 @@
> >  #define NS_NETWORK_LINK_DATA_UNKNOWN "unknown"
> > +
> > +/**
> > + * We send notifications through nsIObserverService with topic
> > + * NS_NETWORK_LINK_TYPE_TOPIC whenever the network conection type
> 
> s/conection/connection/

Thanks, will correct typo and re-attach.

	Chris Lord [:cwiiis]
	Comment 27 • 13 years ago

Comment on attachment 544432 [details] [diff] [review]
Correct typo

Passes on try: http://tbpl.mozilla.org/?tree=Try&rev=28df653f079c

	Doug Turner (:dougt)
	Comment 28 • 13 years ago

http://hg.mozilla.org/integration/mozilla-inbound/rev/49d539befa07

	Mounir Lamouri (:mounir)
	Comment 29 • 13 years ago

Merged:
http://hg.mozilla.org/mozilla-central/rev/49d539befa07

	Matt Brubeck (:mbrubeck)
	Comment 30 • 13 years ago

This will add "Read phone state and identity" to the permissions Firefox requests at install time, right? I think we need a followup bug to consider adding an explanation in our Android Market description of how we use this and other permissions, since it is a common cause of complaints and confusion.

Is the functionality gain from this change worth the loss of automatic updates from Fx7->8, and the perceived privacy issues?

	Doug Turner (:dougt)
	Comment 31 • 13 years ago

> Is the functionality gain from this change worth the loss of automatic updates from Fx7->8, and the perceived privacy issues?

You are correct. I hate the idea of being blocked on adding any new functionality that requires new rights because automatic updates will break.

	Chris Lord [:cwiiis]
	Comment 32 • 13 years ago

I think being able to alter behaviour depending on the type of connection is quite desirable, although it's only the fine-grained type that requires this permission (without, you can determine either 'mobile' or 'wireless', but not what type of mobile or wireless (i.e. wimax, lte, 3g, 2g, etc.)).

It's kind of a shame that the network type isn't retrievable without this permission, as that's a pretty useful piece of information and we don't really need any of the other information it grants access to (yet, at least).

Likelihood is we'll have to add some more permissions in the future for things like Device APIs though, so I don't think this should be held up because of it. Shall I open the bug to add information to our market entry?

	Mike Hommey [:glandium]
	Comment 33 • 13 years ago

(In reply to comment #32)
> I think being able to alter behaviour depending on the type of connection is
> quite desirable, although it's only the fine-grained type that requires this
> permission (without, you can determine either 'mobile' or 'wireless', but
> not what type of mobile or wireless (i.e. wimax, lte, 3g, 2g, etc.)).

I think the main concern of the people Taras mention in comment 0 is that they don't want telemetry to initiate data transfers that will be billed to them.
If there's really a concern about requiring the extra permission, I think the goal of this bug could be very much achieved by only distinguishing between 'mobile' and 'wireless'.

	Doug Turner (:dougt)
	Comment 34 • 13 years ago

I'd like, at some point, to be able to expose network information to webapps.

	Matt Brubeck (:mbrubeck)
	Comment 36 • 13 years ago

(In reply to comment #30)
> This will add "Read phone state and identity" to the permissions Firefox
> requests at install time, right? I think we need a followup bug to consider
> adding an explanation in our Android Market description of how we use this
> and other permissions, since it is a common cause of complaints and
> confusion.

Filed bug 672352.

	Jorge Villalobos [:jorgev] (he/him)
	Comment 37 • 13 years ago

This would be useful to know for add-on developers.

	Trevor
	Comment 38 • 13 years ago

https://developer.mozilla.org/en/XPCOM_Interface_Reference/nsINetworkLinkService

	Matt Brubeck (:mbrubeck)
	Comment 39 • 13 years ago

We are discussing backing this change out; see bug 691054 for details.

	(dormant account) Reporter
	Comment 40 • 13 years ago

(In reply to Matt Brubeck (:mbrubeck) from comment #39)
> We are discussing backing this change out; see bug 691054 for details.

I'm ok with a backout. I was planning to use this for telemetry but the decision of when to not send a telemetry packet on 3g is a complicated one, so we do not need this until we have a solid plan for telemetry.

	Matt Brubeck (:mbrubeck)
	Comment 41 • 13 years ago

This was backed out on all branches because the cost/benefit for adding the new permission was too high (bug 691054).  From bug 691054 comment 14, some possible ways we could re-add this feature (or at least part of it) without the READ_PHONE_STATE permission:

> the original use case (change settings when on a mobile network) could be done with
> ConnectivityManager.getActiveNetworkInfo().getType(), which does not require any scary
> permissions.
> 
> Actually, it looks from the source code like NetworkInfo.getSubType will return the same
> info as TelephonyManager.getNetworkType (although its documentation isn't as good and
> ConnectivityManager might not notify listeners every time the subtype changes), and also
> does not require scary permissions.

	Chris Lord [:cwiiis]
	Comment 42 • 12 years ago

Unassigning in case someone else wants to pick this up.

	Philipp von Weitershausen [:philikon]
	Comment 43 • 12 years ago

JP, can you elaborate on the basecamp nom? AFAICT from the discussion, this bug has been punted on for now. Clearing it for now, please re-nom if appropriate. Thanks.

(Also removing the link to the B2G Network Manager since at least the patch in this bug seems to be Android only.)

	Mounir Lamouri (:mounir)
	Comment 44 • 12 years ago

bug 697355 isn't b2g related AFAICT.

	Lawrence Mandel [:lmandel] (use needinfo)
	Comment 45 • 12 years ago

basecamp+ because we don't want to submit Telemetry data over cellular data connections on B2G. Note that any way to prevent this should be acceptable for v1, including disabling Telemetry on B2G if required. (We can leave the pref but simply not submit Telemetry data until we can limit it to wifi.) Dougt notes that we did not pursue this patch in Android due to the requirement to add a network query permission on app install. Please keep the Android use case in mind when fixing this bug.

	Lawrence Mandel [:lmandel] (use needinfo)
	Comment 46 • 12 years ago

After further discussion during triage, I have created a separate B2G specific bug for this behaviour (bug 813837). I have therefore cleared the basecamp+ flag on this bug.

	Mounir Lamouri (:mounir)
	Comment 47 • 12 years ago

(In reply to Matt Brubeck (:mbrubeck) from comment #41)
> This was backed out on all branches because the cost/benefit for adding the
> new permission was too high (bug 691054).  From bug 691054 comment 14, some
> possible ways we could re-add this feature (or at least part of it) without
> the READ_PHONE_STATE permission:

BTW, I completely forgot to write a comment here about that but in GeckoNetworkManager.java, I am getting the network type and I haven't been asking any permission. It should be easy to change GeckoNetworkManager to return the connection type. I guess all the boilerplate (having the value going from the Java code to the chrome) could be copied from the patch attached in this bug.
If someone is interested to take this bug, I would be glad to give a hand.

	immerfox
	Comment 48 • 12 years ago

I would like to take this bug! Is it possible?

	Mounir Lamouri (:mounir)
	Comment 49 • 12 years ago

You should look at the backed out patches and modify them so they would use mobile/android/base/GeckoNetworkManager.java to get the current connection status.

	Patrick McManus [:mcmanus]
	Comment 51 • 9 years ago

nsinetworklinkservice