-
Notifications
You must be signed in to change notification settings - Fork 514
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
adds Cellular.dataUsage() API #866
Conversation
@@ -74,6 +74,12 @@ class CellularClass : public NetworkClass | |||
|
|||
CellularSignal RSSI(); | |||
|
|||
CellularData dataUsage(void); | |||
|
|||
CellularData dataUsage(const CellularData &data_set); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should return a CellularData&
or it's going to create a new copy of CellularData, which I'm guessing would be counter-intuitive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the returned CellularData is not globally defined and known to the system API, I figured returning a copy was a safe bet. It also sounds like there are return value optimizations that are done that keep things fast. If we return by reference, can you suggest how this might be architected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see it sets the data usage, that wasn't immediately obvious due to my skimming the code and not reading properly. Perhaps a different API name would be helpful? E.g. Cellular.setDataUsage()
. Thinking forward, I imagine we will need data usage APIs for other networks. How about we rename CellularData
to be more network-neutral, e.g simply DataUsage
?
If you could explain the use case for wanting to set the counters to an arbitrary value that might help make it clear to me.
I'm kind of puzzled why we need a return value at all, and what the counter values of the return value are. The counter values are sent into the modem and the result read out from the modem. I'm puzzled by that! Any reason why the returned counters are not the same as the values passed in? If so then we can simply return the original instance as a reference, since the values are the same.
If the returned counters are truly different, then it would be a good idea to update them in the reference passed in (making the reference non-const). Otherwise the user can easily end up losing the changed data by not saving the return value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to start with data = Cellular.dataUsage() as a way to get the current data usage values. Then I added a way to set the values, which I added an overloaded function. It seemed intuitive to pass in a value to set the counters, like creating a String sweet = String("dude")
. And even though we can reset the count now with Cellular.dataUsage(zeroedOutData)
I felt like adding a convenience function just for that was helpful Cellular.dataUsageReset()
opting to add Reset to the end for the day when we have intellitype autocomplete in the IDE :) Of course we could make everything very explicit:
Cellular.dataUsageGet()
Cellular.dataUsageSet()
Cellular.dataUsageReset()
Which wouldn't be so bad. I often try to keep the typing and number of humps in the camelCase to a minimum if possible. It also seems unnecessary to spell everything out when we have a lot of functions in our API like Cellular.RSSI() WiFi.localIP() System.deviceID() etc.. I'm totally open to going one way or the other though if we'd like to define signatures for when either approach is best. Like if an API has set/get functions, make them explicit.
The counter value is saved in the wiring API in a global instance of CellularData, so the object that the user defines in their application.
I chose CellularData
because the values are fairly specific to this modem, and we have a running theme of types for cellular: CellularSignal, CellularDevice, CellularCredentials. You can see there is CID element in CellularData which is the PDP Context index for the data in the counters, fairly specific to cellular. I don't think we need a generic DataUsage object currently, but could maybe see making one in the future that pulls these specific types into it.
I only gave the ability to set the counters to a specific value because the original AT command has this same functionality. I'm guessing this might be appropriate for aggregating a running count over a long period of time. Maybe when a user app wakes up it pulls the values from EEPROM, sets the dataUsage counter and then saves them back before sleeping.
When setting the dataUsage counters, the only thing different in the returned value might be the CID element. It's used as a return value error in the case that the call to the modem failed. So we can't assume the counter object we passed in is has the same values exactly. We could set the values by reference and update the CID value that way as well, I just think this interface changes the way the return value is interpreted from the GET function variant. So coming full circle, I like the simplicity of getting a returned object CellularData from Cellular.dataUsage() that contains all of our counter info, and then to set those values just overloading Cellular.dataUsage(const CellularData&) with the ability to keep our error checking the same way as with the Get function, in the returned object.
I'm not sure that being consistent with the get/set is a virtue - most get/set APIs are asymmetric. Rather than
Isn't this simpler:
and also consistent with our other APIs that return a bool test for failure. We can also do:
Which will bring the get/set APIs into alignment
|
How about we make them all explicit and use pass by reference for set/get, and void for reset. All would return bool based on if the modem query was successful.
|
👍 |
What are your thoughts on integration tests for this, e.g. in the |
It looks like this branch also includes the cellular band select API. Please rebase on develop and force push to see if there are any changes that are missing. |
- set/get now requires CellularData passed by reference - all functions return bool - CellularData is boolean testable, not comparable though on purpose - cellular_hal functions sliced up for unit tests (to be written) - added some API tests
required for this to compile and run `git cherry-pick 3076956` also will merge with cellular.cpp from feature/electron/localip and feature/electron/datausage
added new cellular_internal.h to scr/electron for platform specific function prototypes added Wiring_Cellular conditional compile checks for Cellular added PLATFORM_ID conditional compile checks for platform specific dynalib headers
9baa3ff
to
03b0464
Compare
@m-mcgowan Rebased against develop, resolved a conflict with |
03b0464
to
783a605
Compare
…her than references. The API is a C-API not a C++ API so references are not supported.
adds Cellular.dataUsage() API
A software implementation of Data Usage that pulls sent and received session and total bytes from the cellular modem's internal counters. The sent / received bytes are the gross payload evaluated by the protocol stack, therefore they comprise the TCP and IP header bytes and the packets used to open and close the TCP connection. I.e., these counters account for all overhead in cellular communications.
Note: Cellular.getDataUsage() should only be used for relative measurements on data at runtime. Do not rely upon these figures for absolute and total data usage of your SIM card.
Note: There is a known issue with Sara U260/U270 modem firmware not being able to set/reset the internal counters (which does work on the Sara G350). Because of this limitation, the set/reset function is implemented in software, using the modem's current count as the baseline.
Note: The internal modem counters are typically reset when the modem is powered cycled (complete power removal, soft power down or Cellular.off()) or if the PDP context is deactiaved and reactivated which can happen asynchronously during runtime. If the Cellular.getDataUsage() API has been read, reset or set, and then the modem's counters are reset for any reason, the next call to Cellular.getDataUsage() for a read will detect that the new reading would be less than the previous reading. When this is detected, the current reading will remain the same, and the now lower modem count will be used as the new baseline. Because of this mechanism, it is generally more accurate to read the getDataUsage() count often. This catches the instances when the modem count is reset, before the count starts to increase again.
To use the data usage API, an instance of the
CellularData
type needs to be created to read or set counters. All data usage API functions and the CellularData object itself returnbool
indicating the last operation was successful and the CellularData object was updated. For set and get functions,CellularData
is passed by referenceCellular.dataUsage(CellularData&);
and updated by the function. There are 5 integers and 1 boolean within the CellularData object:false
when the CellularData object is initially created, andtrue
after the object has been successfully updated by the API. If the last reading failed and the counters were not changed from their previous value, this value is set tofalse
.CellularData is a Printable object, so using it directly with
Serial.println(data);
will be output as follows: