Add HL78XX Modem Driver Implementation Using Modem Chat Framework #89541
Conversation
drivers/modem/hl78xx/Kconfig.hl78xx
Outdated
| if MODEM_HL78XX | ||
|
|
||
| choice MODEM_HL78XX_VARIANT | ||
| bool "Sierre Wireless hl781x variant selection" |
There was a problem hiding this comment.
Should be Sierra. Or should we change all references to Semtech now?
There was a problem hiding this comment.
Not quite sure, but Sierre still seems to be good to me.
There was a problem hiding this comment.
I couldn't see any good explanation for this, but the packages and everything that they show it is Sierra Wireless.
they might have kept the copyright on the Sierra Wireless name as well
zafersn
force-pushed
the
hl78xx_driver
branch
2 times, most recently
from
31e6d9f to
b1e6ffc
Compare
There was a problem hiding this comment.
Pull Request Overview
This pull request introduces the initial HL78XX modem driver implementation based on the modem_chat framework while updating sample applications to integrate power management calls. Key changes include:
- Adding power management includes and calls in sample applications to power on the modem.
- Enhancing the modem socket poll routine with temporary POLLOUT handling.
- Introducing new HL78XX utility and header files to support modem functionality.
Reviewed Changes
Copilot reviewed 8 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| samples/net/lwm2m_client/src/lwm2m-client.c | Added power management headers and a power-on invocation for the HL78XX modem. |
| samples/net/cloud/aws_iot_mqtt/src/main.c | Added similar power management initialization for the modem in the AWS IoT MQTT sample. |
| drivers/modem/modem_socket.c | Updated POLLOUT event handling with temporary logging and fallback behavior. |
| drivers/modem/hl78xx/hl78xx_utility.c | Added HL78XX utility functions; note an inconsistent module name in the log registration. |
| drivers/modem/hl78xx/hl78xx.h | Introduced declarations and definitions supporting the HL78XX modem driver. |
Files not reviewed (6)
- drivers/modem/CMakeLists.txt: Language not supported
- drivers/modem/Kconfig: Language not supported
- drivers/modem/hl78xx/CMakeLists.txt: Language not supported
- samples/net/cloud/aws_iot_mqtt/boards/hl78xx_chat.conf: Language not supported
- samples/net/lwm2m_client/Kconfig: Language not supported
- samples/net/lwm2m_client/overlay-hl78xx_chat.conf: Language not supported
Comments suppressed due to low confidence (1)
drivers/modem/hl78xx/hl78xx_utility.c:18
- [nitpick] The module name 'hl7812_utility' appears inconsistent with the HL78XX naming convention used elsewhere. Consider renaming it to 'hl78xx_utility' for naming consistency.
LOG_MODULE_REGISTER(hl7812_utility, CONFIG_MODEM_LOG_LEVEL);
c4e404b to
df0f647
Compare
zafersn
force-pushed
the
hl78xx_driver
branch
8 times, most recently
from
070889d to
a7f1641
Compare
Adding HL78XX Modem Driver Implementation Using Modem Chat Framework Signed-off-by: Zafer SEN <zafersn93@gmail.com>
zafersn
force-pushed
the
hl78xx_driver
branch
4 times, most recently
from
416d248 to
e8af631
Compare
Add HL78xx driver sample application to show hl78xx user APIs & Configs Signed-off-by: Zafer SEN <zafersn93@gmail.com>
|
|
I added an event notifier/dispatcher to let the app layer know when something happens with the modem, like different events or status changes. I included a basic example of how this works in the sample app here: zephyr/samples/drivers/modem/hello_hl78xx/src/main.c Lines 112 to 133 in 767737a Originally, I planned to replicate AT_MONITOR from the nRF SDK (https://github.com/nrfconnect/sdk-nrf/blob/main/lib/at_monitor/at_monitor.c) and just forward all URCs (unsolicited result codes)/ at cmd traffics to the app layer directly. But that would have required changes in the modem_chat driver, which I wanted to avoid. So instead, I went with a cleaner approach, the event notifier. Now, whenever the modem reports something like registration status changes, PSM updates, RAT mode changes, etc., those events get pushed to all registered listeners. |
|
|
||
| choice MODEM_HL78XX_VARIANT | ||
| bool "Sierra Wireless hl78xx variant selection" | ||
| default MODEM_HL7812 |
There was a problem hiding this comment.
Wouldn't MODEM_HL78XX_AUTODETECT_VARIANT be a better default here?
| help | ||
| The address family for IP connections. |
There was a problem hiding this comment.
| help | |
| The address family for IP connections. | |
| help | |
| The address family for IP connections. |
| help | ||
| - Keep the device in boot mode untiL have +CREG/+CEREG: 1(normal) or 5(roaming) | ||
| endif # MODEM_HL78XX_BOOT_IN_FULLY_FUNCTIONAL_MODE |
There was a problem hiding this comment.
| help | |
| - Keep the device in boot mode untiL have +CREG/+CEREG: 1(normal) or 5(roaming) | |
| endif # MODEM_HL78XX_BOOT_IN_FULLY_FUNCTIONAL_MODE | |
| help | |
| Keep the device in boot mode untiL have +CREG/+CEREG: 1(normal) or 5(roaming) | |
| endif # MODEM_HL78XX_BOOT_IN_FULLY_FUNCTIONAL_MODE |
| } | ||
|
|
||
| /* Initialize during SYS_INIT */ | ||
| SYS_INIT(hl78xx_evt_monitor_sys_init, APPLICATION, 0); |
There was a problem hiding this comment.
Maybe add a kconfig for application level priority?
| } | ||
| void hl78xx_on_kstatev_parser(struct hl78xx_data *data, int state) | ||
| { |
There was a problem hiding this comment.
Check clang formatting on all files.
| } | |
| void hl78xx_on_kstatev_parser(struct hl78xx_data *data, int state) | |
| { | |
| } | |
| void hl78xx_on_kstatev_parser(struct hl78xx_data *data, int state) | |
| { |
| } | ||
| static bool parse_ip(bool is_ipv4, const char *ip_str, void *out_addr) | ||
| { | ||
| int ret = net_addr_pton(is_ipv4 ? AF_INET : AF_INET6, ip_str, out_addr); |
There was a problem hiding this comment.
| } | |
| static bool parse_ip(bool is_ipv4, const char *ip_str, void *out_addr) | |
| { | |
| int ret = net_addr_pton(is_ipv4 ? AF_INET : AF_INET6, ip_str, out_addr); | |
| } | |
| static bool parse_ip(bool is_ipv4, const char *ip_str, void *out_addr) | |
| { | |
| int ret = net_addr_pton(is_ipv4 ? AF_INET : AF_INET6, ip_str, out_addr); |
| } | ||
| void iface_status_work_cb(struct hl78xx_data *data, modem_chat_script_callback script_user_callback) |
There was a problem hiding this comment.
| } | |
| void iface_status_work_cb(struct hl78xx_data *data, modem_chat_script_callback script_user_callback) | |
| } | |
| void iface_status_work_cb(struct hl78xx_data *data, modem_chat_script_callback script_user_callback) |
| } | ||
| void dns_work_cb(void) | ||
| { |
There was a problem hiding this comment.
| } | |
| void dns_work_cb(void) | |
| { | |
| } | |
| void dns_work_cb(void) | |
| { |
| } | ||
| /* send binary data via the +KUDPSND commands */ | ||
| static ssize_t send_socket_data(void *obj, const struct sockaddr *dst_addr, const char *buf, | ||
| size_t buf_len, k_timeout_t timeout) |
There was a problem hiding this comment.
| } | |
| /* send binary data via the +KUDPSND commands */ | |
| static ssize_t send_socket_data(void *obj, const struct sockaddr *dst_addr, const char *buf, | |
| size_t buf_len, k_timeout_t timeout) | |
| } | |
| /* send binary data via the +KUDPSND commands */ | |
| static ssize_t send_socket_data(void *obj, const struct sockaddr *dst_addr, const char *buf, | |
| size_t buf_len, k_timeout_t timeout) |
| } | ||
| static ssize_t offload_sendto(void *obj, const void *buf, size_t len, int flags, | ||
| const struct sockaddr *to, socklen_t tolen) |
There was a problem hiding this comment.
| } | |
| static ssize_t offload_sendto(void *obj, const void *buf, size_t len, int flags, | |
| const struct sockaddr *to, socklen_t tolen) | |
| } | |
| static ssize_t offload_sendto(void *obj, const void *buf, size_t len, int flags, | |
| const struct sockaddr *to, socklen_t tolen) |
| CONFIG_TEST_RANDOM_GENERATOR=y | ||
| CONFIG_INIT_STACKS=y | ||
| CONFIG_HW_STACK_PROTECTION=y | ||
| CONFIG_REQUIRES_FULL_LIBC=y |
There was a problem hiding this comment.
This feels overly complicated for such a simple sample. Can this be optimized to only include options that are absolutely necessary?
| pm_device_action_run(modem, PM_DEVICE_ACTION_RESUME); | ||
| #endif | ||
|
|
||
| #ifdef CONFIG_MODEM_HL78XX_BOOT_IN_FULLY_FUNCTIONAL_MODE |
There was a problem hiding this comment.
What about other boot modes? How can they be used? Can the user then change at runtime?
This pull request introduces the initial draft implementation of the HL78XX modem driver, built on the
modem_chatframework. The current version establishes the core structure and essential functionality required to support the HL78XX modem series.The primary objective at this stage is to review and align on the overall design and integration approach. Following this, development will continue with additional features such as:
Power saving mode support
User pipe connection handling
Other modem-specific enhancements
Verified compatibility and correct operation with the following sample applications:
aws_mqttdns_resolvebig_http_downloadlwm2mFeedback on both structure and test approach is welcome.