Skip to content

update schemas to reflect what asset_tracker_v2 is actually sending #51

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 14 commits into
base: v1
Choose a base branch
from

Conversation

coderbyheart
Copy link
Member

@coderbyheart coderbyheart commented Feb 28, 2023

This aligns the schemas to match the data sent by asset_tracker_v2.

Question remains where this mismatch should be fixed, it could also be fixed on the firmware side.

@@ -54,9 +54,9 @@
"type": "integer"
},
"mccmnc": {
"type": "string",
"type": "integer",
Copy link
Contributor

Choose a reason for hiding this comment

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

https://github.com/nrfconnect/sdk-nrf/blob/3797fac81db0b317d14abded836b3ebc3220979c/lib/modem_info/modem_info_params.c#L71

this has been a string for a long time.
ATv2 has its own implementation, which is a number.
i have no preference, other than that all code in NCS is aligned.

@@ -127,12 +128,13 @@
},
"appName": {
"type": "string"
},
"iccid": {
Copy link
Contributor

Choose a reason for hiding this comment

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

https://github.com/nrfconnect/sdk-nrf/blob/3797fac81db0b317d14abded836b3ebc3220979c/lib/modem_info/modem_info_params.c#L39

ICCID, along with IMSI and UICC mode, has been reported in the "simInfo" section for a long time.
again, no preference, but all NCS code should be aligned.

@coderbyheart coderbyheart force-pushed the v1-asset_tracker_v2-fixes branch 2 times, most recently from 00917f7 to e32d679 Compare April 20, 2023 12:29
@@ -171,9 +171,9 @@
"type": "integer"
},
"mccmnc": {
"type": "string",
"type": "number",
Copy link
Contributor

Choose a reason for hiding this comment

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

the modem_info library provides a string for this value.
the nrf_cloud library reports this value as a string.

Copy link
Member Author

Choose a reason for hiding this comment

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

Current asset_tracker_v2 sends number:

"roam": {
          "v": {
            "band": 12,
            "nw": "LTE-M",
            "rsrp": -119,
            "area": 34635,
            "mccmnc": 310410,
            "cell": 141255696,
            "ip": "10.165.115.123",
            "eest": 7
          },
          "ts": 1681909040890
        },

Copy link
Contributor

Choose a reason for hiding this comment

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

this is a breaking change for fw apps using modem_info library or nrf_cloud library to update the shadow.

@theflintstone
Copy link

@coderbyheart. The NCS change notes says that you provide BATTERY and not VOLTAGE appid. I do not see that change here? Or should there be new PR

Also, the BATTERY level seems to be from PMIC. Does all our Thingys and DKs contain the needed PMIC?

@coderbyheart
Copy link
Member Author

@coderbyheart. The NCS change notes says that you provide BATTERY and not VOLTAGE appid. I do not see that change here? Or should there be new PR

I haven't added this, yet.

Also, the BATTERY level seems to be from PMIC. Does all our Thingys and DKs contain the needed PMIC?

I don't know.

@coderbyheart coderbyheart force-pushed the v1-asset_tracker_v2-fixes branch from b3598a5 to 37ac0f3 Compare May 30, 2023 13:52
@coderbyheart
Copy link
Member Author

Added.

@coderbyheart coderbyheart force-pushed the v1-asset_tracker_v2-fixes branch from 17e75c2 to 5ba9b31 Compare November 9, 2023 13:29
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.

3 participants