Add new APIs to access device via control requests

[keeramis]

Description

This PR introduces device API calls to get the network interfaces and cell radio IMEI

How to test

Please see : particle-iot/particle-cli#713

References

Particle-CLI : particle-iot/particle-cli#713
Device-OS : particle-iot/device-os#2730
Particle-USB : particle-iot/device-os-protobuf#26

[monkbroc]

I'll let you address the comments by Sergey to parse the responses in the library and expose clean objects, then I'll review

[keeramis]

For a device’s network interface list, It appears that this field source is missing from the ipv4Config (checked on P2 and Argon). Device-os is not populating this field device-os link. Hope I am not missing anything here.
For now though, I skipped using this field which we expected to use as a validation check for the ipv*Config.

[monkbroc]

Then remove the if because the the ipv4Config object will always be defined. This is because it contains arrays, that if not sent by the device will be set as empty arrays by the protobuf decoder.

[keeramis]

@monkbroc I would leave this here, so I can use this check to call convertIP4Address or convertIP6Address to parse the ipv4 or ipv6 formatted address.

[keeramis]

For line 46, if (ifaceAddr && ifaceAddr.address && ifaceAddr.address[version]) {.

I tried using the optional chaining ?. but the eslint fails (even with v7.15.0 which is expected to support eslint as per this doc https://eslint.org/docs/latest/rules/no-unsafe-optional-chaining).

[monkbroc]

Most of our projects use the eslint-config-particle package to share config. We haven't updated this to allow optional chaining. That's on the todo list. There's a workaround to update the eslint config in this project directly, but probably not worth it for one or two statements.

[sergeuz]

Thinking about it now, how about we use a more generic name for this method, e.g. getCellularInfo()? Just so that it could potentially be extended with more info later? @monkbroc

[sergeuz]

Nit: I don't think neither address nor prefixLength can be missing as long as ifactAddress is not null, but if we explicitly check for optionality of address we should do so for prefixLength as well. Currently, this function will return something like X.X.X.X/null if only address is available.

[monkbroc]

Suggested change

	'address': 3232257567
	'v4': 3232257567

[keeramis]

message Ipv4Config {
  repeated InterfaceAddress addresses = 1;
  // On P2P links
  Ipv4Address peer = 2;
  // Temporary, will be moved to routing table
  Ipv4Address gateway = 3 [(nanopb).proto3 = false];

  repeated Ipv4Address dns = 4;
  InterfaceConfigurationSource source = 5;
}

A dns is an Ipv4address which is

message Ipv4Address {
  fixed32 address = 1;
}

Why are we adding the v4 here? The version string as key is for the IPAddress which is only for the InterfaceAddress

[sergeuz]

I think part of this confusion stems from the fact that there's a single function in the code, convertIPAddress, that can convert both an instance of Protobuf's Ipv4Address and Ipv6Address even though they're unrelated classes. For better clarity, I would have separate functions in the code defined as follows:

• convertIP4Address – converts a Protobuf's Ipv4Address to a string (also used directly when converting addresses from Ipv4Config).
• convertIP6Address – converts a Protobuf's Ipv6Address to a string (also used directly when converting addresses from Ipv6Config).
• convertIPAddress – converts Protobuf's IpAddress to a string. Decides whether the address is v4 or v6 based on the message structure and uses either convertIP4Address or convertIP6Address internally so that there's no need to pass the version as an argument in the code.

[keeramis]

@sergeuz The main question perhaps here is the structure of the dns, gateway, and peer addresses.

Will it look like this:

dns: [
    { address: xxx }
],
gateway: { address: ggg }
peer: { address: ppp }

OR this:

dns: [
    xxx
],
gateway: ggg
peer: ppp

[sergeuz]

Will it look like this

If you mean an object returned by the particle-usb API, then it should look like in the second example.

[keeramis]

I agree with the implementation 👍

If possible, could you confirm if this message structure is correct, esp for the ipv4Config.dns?

particle-usb/src/network-device.test.js

Lines 45 to 84 in 14c2cd6

	const input = {
	'index': 4,
	'name': 'wl3',
	'type': 8,
	'ipv4Config': {
	'dns': [
	{
	'address': 3232257567
	}
	],
	'addresses': [
	{
	'address': {
	'v4': {
	'address': 3232257567
	}
	},
	'prefixLength': 24
	}
	]
	},
	'ipv6Config': {
	'dns': [
	{
	'address': Buffer.from([2, 1, 4, 3, 6, 5, 8, 7, 10, 9, 12, 11, 14, 13, 15])
	}
	],
	'addresses': [
	{
	'address': {
	'v6': {
	'address': Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15])
	}
	},
	'prefixLength': 24
	}
	]
	},
	'hwAddress': Buffer.from([48,174,164,229,83,16])
	};

[sergeuz]

Yes, that's what I would expect it to look like based on the message structure in the .proto files.

[keeramis]

Awesome! Thanks for confirming the format. I will make the implementation change per your suggestion

[sergeuz]

It's not ideal that we can't test it with a real device, but as long as the function that parses an IpAddress works correctly (convertIPAddress from the list), we can expect that convertIP4Address and convertIP6Address will work correctly too since those will be used internally by convertIPAddress.

[keeramis]

That's how I was able to deduce the format. Via the output of the InterfaceAddress of a real device.

[monkbroc]

I'm done tweaking this :) I say let's merge ahead