-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
drivers: Add cellular API for network configuration #64537
Closed
bjarki-andreasen
wants to merge
1
commit into
zephyrproject-rtos:main
from
bjarki-andreasen:cellular_api_network_config
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,101 @@ | ||
/* | ||
* Copyright (c) 2023 Bjarki Arge Andreasen | ||
* | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
/** | ||
* @file drivers/cellular.h | ||
* @brief Public cellular network API | ||
*/ | ||
|
||
#ifndef ZEPHYR_INCLUDE_DRIVERS_CELLULAR_H_ | ||
#define ZEPHYR_INCLUDE_DRIVERS_CELLULAR_H_ | ||
|
||
/** | ||
* @brief Cellular interface | ||
* @defgroup cellular_interface Cellular Interface | ||
* @ingroup io_interfaces | ||
* @{ | ||
*/ | ||
|
||
#include <zephyr/types.h> | ||
#include <zephyr/device.h> | ||
#include <errno.h> | ||
|
||
#ifdef __cplusplus | ||
extern "C" { | ||
#endif | ||
|
||
/** Cellular access technologies */ | ||
enum cellular_access_technology { | ||
CELLULAR_ACCESS_TECHNOLOGY_GSM = 0, | ||
CELLULAR_ACCESS_TECHNOLOGY_GPRS, | ||
CELLULAR_ACCESS_TECHNOLOGY_UMTS, | ||
CELLULAR_ACCESS_TECHNOLOGY_EDGE, | ||
CELLULAR_ACCESS_TECHNOLOGY_LTE, | ||
CELLULAR_ACCESS_TECHNOLOGY_LTE_CAT_M1, | ||
CELLULAR_ACCESS_TECHNOLOGY_LTE_CAT_M2, | ||
CELLULAR_ACCESS_TECHNOLOGY_NB_IOT, | ||
}; | ||
|
||
/** Cellular network structure */ | ||
struct cellular_network { | ||
/** Cellular access technology */ | ||
enum cellular_access_technology technology; | ||
/** | ||
* List of bands, as defined by the specified cellular access technology, | ||
* to enables. All supported bands are enabled if none are provided. | ||
*/ | ||
uint16_t *bands; | ||
/** Size of bands */ | ||
uint16_t size; | ||
}; | ||
|
||
/** | ||
* @brief Configure cellular networks for the device | ||
* | ||
* @details Cellular network devices support at least one cellular access technology. | ||
* Each cellular access technology defines a set of bands, of which the cellular device | ||
* will support all or a subset of. | ||
* | ||
* The cellular device can only use one cellular network technology at a time. It must | ||
* exclusively use the cellular network configurations provided, and will prioritize | ||
* the cellular network configurations in the order they are provided in case there are | ||
* multiple (the first cellular network configuration has the highest priority). | ||
* | ||
* @param dev Cellular network device instance. | ||
* @param networks List of cellular network configurations to apply. | ||
* @param size Size of list of cellular network configurations. | ||
* | ||
* @retval 0 if successful. | ||
* @retval -EINVAL if any provided cellular network configuration is invalid or unsupported. | ||
* @retval -ENOTSUP if API is not supported by cellular network device. | ||
* @retval Negative errno-code otherwise. | ||
*/ | ||
int cellular_configure_networks(const struct device *dev, | ||
const struct cellular_network *networks, uint8_t size); | ||
|
||
/** | ||
* @brief Get supported cellular networks for the device | ||
* | ||
* @param dev Cellular network device instance | ||
* @param networks Pointer to list of supported cellular network configurations. | ||
* @param size Size of list of cellular network configurations. | ||
* | ||
* @retval 0 if successful. | ||
* @retval -ENOTSUP if API is not supported by cellular network device. | ||
* @retval Negative errno-code otherwise. | ||
*/ | ||
int cellular_get_supported_networks(const struct device *dev, | ||
const struct cellular_network **networks, uint8_t *size); | ||
|
||
#ifdef __cplusplus | ||
} | ||
#endif | ||
|
||
/** | ||
* @} | ||
*/ | ||
|
||
#endif /* ZEPHYR_INCLUDE_DRIVERS_CELLULAR_H_ */ |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 seem to mean that user or the API needs to allocate the band array from heap. Could we avoid that, what about setting an array here with max number of bands, or could we use a bitmask to define the bands?
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.
We will run into issues as there are band numbers that exceed 255 already n256 so we would need 5 uint64_t to cover it with a bitmask.
Usually, a user would only enable a few specific bands, and they are very specific to the region (available operators), so the list would not be that long if it is provided. Most will not provide the bands at all which is why it is optional :)
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.
Would you prefer setting enabling/disabling bands one at a time maybe?
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 would just like to avoid using dynamic memory allocations here because of possible memory leaks.
About the band numbers, are they pre-defined? If so, then could you do
If not, then that is probably not an option.
If the number of bands is small, could we do instead?
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.
The first option is not a possibility due to the number of bands per network technology :(
Regarding the fixed size array, I understand the concern regarding the potential memory unsafety, but it is not more unsafe than
write(struct context *ctx, uint8_t *data, size_t size)
, which is a generally accepted pattern for non-fixed sized data. The bands do not need to be allocated on the heap, they can, and will most likely, be statically allocated or on the stack, even const, consider:The modems support a fixed set of technologies and bands, so these will be const and statically defined, which is why the
cellular_get_supported_networks
function provides a pointer to a const networks array :) We can even create a macro to do this which increases safety:which expands to
the va args being the bands
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.
A fixed size array will result in wasted memory given it has to fit the largest set of bands, which is not desired :)
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 might have missed how the api is used, could you add a sample or test to show the intended usage.
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.
The driver will create an array of supported networks based on the modem datasheet. The datasheet will provide a list like this
Which will then be stored in ROM, within the driver, like:
The application can then get the pointer to this const array using the
cellular_get_supported_networks()
function. The supported bands are like flash pages, stored statically and most likely const, (at the very least initialized in the modem init function)To configure the modem to use LTE_CAT_M1, and GSM only, allowing all bands,, the application will do
In this case the networks only needs to be valid until the function returns, like when writing data for example