-
Notifications
You must be signed in to change notification settings - Fork 2
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
samples: dect: add simple sample #136
Conversation
ab567a5
to
75b84b3
Compare
72144f1
to
4eb86e9
Compare
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.
Did a first pass
samples/dect/dect_phy_simple/Kconfig
Outdated
|
||
config CARRIER | ||
int "Carrier to use" | ||
default 1677 #during development, remove default from sample so cutomer must address it?- |
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 number depends on the local regulations.. perhaps we should not have compile out of the box, since we don't know which country the customer is. We could have an overlay for Europe and one for USA maybe with different values for this KConfig
that are ready to use, but require a manual configuration step by the customer.
samples/dect/dect_phy_simple/Kconfig
Outdated
default 1677 #during development, remove default from sample so cutomer must address it?- | ||
help | ||
The availability of the channels and the exact regulation to use them varies in different countries. | ||
See ETSI TS 103 636-2 5.4.2 for the calculation. |
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.
Table 4.3.2.3-2 of ETSI EN 301 406-2
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.
Or actually, nevermind that. ETSI EN
applies to Europe only.. better to refer to the TS
document here I suppose.
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.
let's call it something else than simple
maybe hello_dect
?
return 0; | ||
} | ||
|
||
SHELL_CMD_REGISTER(exit, NULL, "Download modem firmware", shell_exit); |
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.
exit
.. Download modem firmware
?
struct nrf_modem_dect_phy_rx_params rxOpsParams = { 0 }; | ||
|
||
rxOpsParams.start_time = 0; |
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.
do this instead:
struct foo = {
.bar = 1,
.baz = 2,
}
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 think splitting the sample in two RX/TX samples would simplify it quite a bit.. In particular we avoid having to configure a board as transmitter or receiver, which here is done with DK buttons. I'd rather not use any buttons and LEDs at all to be honest.
Ideally the transmitter would send "Hello world, DECT" and a counter, and the receiver would just display that on the console.
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 board is not configured with the DK button, those are only used to exit the sample. The first device will take the TX role as it does not receive anything.
We could split it so that we have two samples too, though it would be some duplicate code and the user would have to build and flash two different applications.
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.
Indeed, the buttons and LEDs are not for configuring the sample; that was my misunderstanding. The sample self-configures through a different method. Regardless, if the logic was hardcoded, it would simplify understanding the sample. Building and flashing two distinct samples is necessary, though I believe it's not a big deal..
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.
Also, I think the sample should run some configurable number of iterations and then exit by itself (no buttons/shell). Optionally, the number of iterations can be some special value indicating "don't exit".
FILE(GLOB app_sources src/main.c) | ||
target_sources(app PRIVATE ${app_sources}) |
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.
just add main.c
with target_sources()
4eb86e9
to
95ee4a7
Compare
1. SDC_HCI_OPCODE_CMD_VS_SCAN_CHANNEL_MAP_SET This command can be used to set the primary advertising channels that should be used for scanning and initiating. 2. SDC_HCI_OPCODE_CMD_VS_SCAN_ACCEPT_EXT_ADV_PACKETS_SET This command enables/disables processing of extended advertising PDUs. Signed-off-by: Johan Stridkvist <johan.stridkvist@nordicsemi.no>
Commit sets adjusted values for `OPENTHREAD_PLATFORM_CSL_UNCERT` and `IEEE802154_NRF5_DELAY_TRX_ACC`. Signed-off-by: Przemyslaw Bida <przemyslaw.bida@nordicsemi.no>
Add a Features of nRF53 Series page Signed-off-by: Katherine Depa <katherine.depa@nordicsemi.no>
Add service for managing SUIT-based updates. Ref: NCSDK-26629 Signed-off-by: Tomasz Chyrowicz <tomasz.chyrowicz@nordicsemi.no>
adds ns target based on crypto samples. Signed-off-by: Anna Wojdylo <anna.wojdylo@nordicsemi.no>
This fixes a few bugs where cadence-related functions would attempt to work with uninitalized sensor values causing null pointer dereferencing. This makes it so that: * Cadence updates before cadence is initialized are ignored * The first delta check before any `prev` value is available will always return `true`. Also adds a null check to `bt_mesh_sensor_value_get_status` by using the API function for float conversion (which already does a NULL check) instead of the bare type callback function. Signed-off-by: Ludvig Jordet <ludvig.jordet@nordicsemi.no>
Use suit-processor revision, compatible with ZCBOR 0.8.1. Ref: NCSDK-26629 Signed-off-by: Tomasz Chyrowicz <tomasz.chyrowicz@nordicsemi.no>
Enable SUIT subsystem unit tests. Ref: NCSDK-26629 Signed-off-by: Tomasz Chyrowicz <tomasz.chyrowicz@nordicsemi.no>
95ee4a7
to
4a098bf
Compare
I've added more alternatives to the sample now:
Please have a look what approach to go for. |
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.
First cursory look
|
||
The sample shows a simple broadcast of DECT NR+ messages from one device to other devices on a hard-coded channel. | ||
|
||
During initialization the devices listens on the channel for broadcast transmissions. |
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.
During initialization the devices listens on the channel for broadcast transmissions. | |
During initialization the devices listen on the channel for broadcast transmissions. |
The sample shows a simple broadcast of DECT NR+ messages from one device to other devices on a hard-coded channel. | ||
|
||
During initialization the devices listens on the channel for broadcast transmissions. | ||
Dependent on the detection of transmissions, the device takes one of the following roles: |
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.
Dependent on the detection of transmissions, the device takes one of the following roles: | |
Depending on the detection of transmissions, the device takes one of the following roles: |
During initialization the devices listens on the channel for broadcast transmissions. | ||
Dependent on the detection of transmissions, the device takes one of the following roles: | ||
|
||
* If no transmission is detected, the device takes the role as a tranceiver. In this role, the device sends an incrementing counter value on the hard-coded channel. The period between each transmission is decided by the :kconfig:option:`CONFIG_TX_INTERVAL_SECONDS` Kconfig option. |
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.
* If no transmission is detected, the device takes the role as a tranceiver. In this role, the device sends an incrementing counter value on the hard-coded channel. The period between each transmission is decided by the :kconfig:option:`CONFIG_TX_INTERVAL_SECONDS` Kconfig option. | |
* If no transmission is detected, the device takes the role of a tranceiver. In this role, the device sends an incrementing counter value on the hard-coded channel. The period between each transmission is decided by the :kconfig:option:`CONFIG_TX_INTERVAL_SECONDS` Kconfig option. |
Dependent on the detection of transmissions, the device takes one of the following roles: | ||
|
||
* If no transmission is detected, the device takes the role as a tranceiver. In this role, the device sends an incrementing counter value on the hard-coded channel. The period between each transmission is decided by the :kconfig:option:`CONFIG_TX_INTERVAL_SECONDS` Kconfig option. | ||
* If a transmission is detected, the device takes the role as a receiver. In this role, the device listens for incomming messages. |
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.
* If a transmission is detected, the device takes the role as a receiver. In this role, the device listens for incomming messages. | |
* If a transmission is detected, the device takes the role of a receiver. In this role, the device listens for incoming messages. |
return err; | ||
} | ||
|
||
k_sem_take(&modem_sem, K_FOREVER); |
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.
Please add some comments explaining what it is blocking on and what it is unblocking (next line). It might get confusing in my opinion since there are many moving parts sharing this semaphore.
return -EIO; | ||
} | ||
|
||
LOG_INF("Dect NR+ PHY initialized %d", err); |
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.
Is logging err
necessary here even in happy path?
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 is removed.
} | ||
|
||
err = modem_rx(NRF_MODEM_DECT_PHY_RX_MODE_SINGLE_SHOT, | ||
CONFIG_RX_TIMEOUT_S * MSEC_PER_SEC, handle); |
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.
Some alignments problems, at least on GitHub.
}; | ||
|
||
/* Receive operation, listen for listen_time_ms duration */ | ||
int modem_rx(uint32_t rxMode, int listen_time_ms, uint32_t handle) |
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.
Why camelcase rxMode
? Also camelcase rxOpsParams
later on.
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.
left from earlier versions, I will correct for the sample(s) we end up with.
void capability_get(const uint64_t *time, enum nrf_modem_dect_phy_err err, | ||
const struct nrf_modem_dect_phy_capability *capability) | ||
{ | ||
LOG_DBG("capability_get time %"PRIu64" Status %d", *time, err); |
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.
Is time
always non-null or is there a chance that it might be null?
void pdc_crc_err( | ||
const uint64_t *time, const struct nrf_modem_dect_phy_rx_pdc_crc_failure *crc_failure) | ||
{ | ||
int16_t resp=calcRSSI(crc_failure->rssi_2); |
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.
int16_t resp=calcRSSI(crc_failure->rssi_2); | |
int16_t resp = calcRSSI(crc_failure->rssi_2); |
K_SEM_DEFINE(rx_sem, 0, 1); | ||
|
||
/* ETSI TS 103 636-2 spec 8.3.3 RSSI is reported every 0.5dbm */ | ||
int32_t calcRSSI(int16_t recrssi){ |
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.
Camelcase? Is this copied from somewhere else?
1c0548f
to
7cc0a6d
Compare
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 notice that there is a lot of shared code between the samples.
k_sem_give(&tx_sem); | ||
} | ||
|
||
K_TIMER_DEFINE(tx_timer, tx_timer_callback, NULL); |
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.
Could be moved up together with the other defines.
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 needs to be defined after the callback.
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 callback can be declared on top and defined 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.
True, though I don't see the need when the function is this small and there is different type of defines.
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've restructured a bit in the _rxtx sample.
K_TIMER_DEFINE(tx_timer, tx_timer_callback, NULL); | ||
|
||
/* ETSI TS 103 636-2 spec 8.3.3 RSSI is reported every 0.5dbm */ | ||
int32_t calcRSSI(int16_t recrssi){ |
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 see that this is shared with other samples. Perhaps there is a way to have a common implementation for code reuse?
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 don't think that is a good idea for samples as it often becomes messy over time and less easy to get an overview. If there is features that should be split out it should be in a library. In this case, I think we should go with the _txrx sample, which will remove this issue.
int err; | ||
uint32_t tx_counter_value = 0; | ||
|
||
LOG_INF("Dect NR+ PHY sample started"); |
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 specify here which variant of the sample this is (TX).
if (exit) { | ||
return -EIO; | ||
} | ||
|
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.
Extra newline.
|
||
|
||
LOG_INF("Dect NR+ PHY initialized %d", err); | ||
|
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.
Extra newline.
err = transmit_counter(tx_counter_value); | ||
if (err) { | ||
LOG_ERR("transmit failed, err %d", err); | ||
continue; |
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.
Is there a chance this might fail repeatedly if there is no backoff in between retransmissions?
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.
If this fails it is due to an issue in the modem library or misuse by the sample. This returns as soon as the request is sent to the modem, and will synchronize internally with other modem calls.
continue; | ||
} | ||
|
||
LOG_INF("Data sent %d",tx_counter_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.
LOG_INF("Data sent %d",tx_counter_value); | |
LOG_INF("Data sent %d", tx_counter_value); |
Pull in a fix in pinctrl that prevents the nrf_qspi_nor driver from failing to initialize when the flash chip is configured to work in non-Quad mode. Signed-off-by: Andrzej Głąbek <andrzej.glabek@nordicsemi.no>
Support WPA Auto security mode wherein supplicant can implicitly choose the highest security among WPA, WPA2 and WPA3 for association with a network based on network support. Fixes SHEL-1131. Signed-off-by: Ravi Dondaputi <ravi.dondaputi@nordicsemi.no>
The `connect` command has been updated to take key-value style arguments. Update the description to include this information. Fixes SHEL-2699. Signed-off-by: Ravi Dondaputi <ravi.dondaputi@nordicsemi.no>
To limit integration scope. For: zephyr/tests/subsys/mgmt/mcumgr/os_mgmt_info zephyr/tests/subsys/mgmt/mcumgr/fs_mgmt_hash_supported zephyr/tests/subsys/mgmt/mcumgr/smp_version zephyr/tests/subsys/mgmt/mcumgr/os_mgmt_echo zephyr/samples/subsys/mgmt/mcumgr/smp_svr nrf/tests/subsys/dfu/dfu_target_stream Signed-off-by: Piotr Kosycarz <piotr.kosycarz@nordicsemi.no>
Compilation with nrf7002ek configuration had less than 200 bytes of SRAM free, which didn't allow much room for own tweaks. It's evident this would overflow in the near future so decreasing the number of requested WiFi APs from 30 to 20. It could be even 10 like with Thingy91X configuration. Signed-off-by: Tommi Rantanen <tommi.rantanen@nordicsemi.no>
Thingy91x has some configurations that doesn't really need to be supported and they do not fit to flash. Removing them. Signed-off-by: Tommi Rantanen <tommi.rantanen@nordicsemi.no>
Update sdk-zephyr to pull in several commits cherry-picked from upstream. Signed-off-by: Jędrzej Ciupis <jedrzej.ciupis@nordicsemi.no>
Add basic sample that demonstrates SUIT-based boot and update procedures. Ref: NCSDK-26629 Signed-off-by: Tomasz Chyrowicz <tomasz.chyrowicz@nordicsemi.no>
Require at least 3 backups to encode SUIT SSF responses and enforce canonical en/decoding if the SUIT-enabled SDFW is used. Ref: NCSDK-26629 Signed-off-by: Tomasz Chyrowicz <tomasz.chyrowicz@nordicsemi.no>
Add a sleep to address the issue where the sample doesn't wait for the WPA supplicant to be fully initialized before proceeding with Wi-Fi operations. Signed-off-by: Triveni Danda <triveni.danda@nordicsemi.no>
The new Zephyr revision introduces experimental EXMIF peripheral driver. Signed-off-by: Rafał Kuźnia <rafal.kuznia@nordicsemi.no>
The compatible nordic,nrf2220-fem is added to bindings. Signed-off-by: Andrzej Kuros <andrzej.kuros@nordicsemi.no>
This commit adds initial experimental support for nrf2220 Front-End device. Signed-off-by: Andrzej Kuros <andrzej.kuros@nordicsemi.no>
This commit adds initial support for nrf2220 Front-End device through fem_al API. Signed-off-by: Andrzej Kuros <andrzej.kuros@nordicsemi.no>
This commit adds initial support for the nrf2220ek shield. Signed-off-by: Andrzej Kuros <andrzej.kuros@nordicsemi.no>
The compatible nordic,nrf2240-fem is added to bindings. Signed-off-by: Andrzej Kuros <andrzej.kuros@nordicsemi.no>
This commit adds initial experimental support for nrf2240 Front-End device. Signed-off-by: Andrzej Kuros <andrzej.kuros@nordicsemi.no>
This commit adds initial support for nrf2240 Front-End device through fem_al API. Signed-off-by: Andrzej Kuros <andrzej.kuros@nordicsemi.no>
This commit adds initial support for the nrf2240ek shield. Signed-off-by: Andrzej Kuros <andrzej.kuros@nordicsemi.no>
The nrf2220 and nrf2240 devices require that mpsl_fem_init operation happens before the initialization of I2C if the TWI interface is used with these devices. This was achieved by hardcoded 49 value which is less than default value of Kconfig option I2C_INIT_PRIORITY. This commit removes the magic value form SYS_INIT(..., 49) and introduces a new Kconfig option MPSL_FEM_INIT_PRIORITY. The default value for MPSL_FEM_INIT_PRIORITY in case of nrf2220 or nrf2240 with TWI is still hardcoded to 49 due to following limitations: 1) The Kconfig compiler does not allow compile-time calculation of default value like: config MPSL_FEM_INIT_PRIORITY default (I2C_INIT_PRIORITY-1) if ... 2) The third parameter of SYS_INIT must be a literal or come from a macro expanding to literal. It cannot expand to expression. So following is impossible: #define FEM_INIT_PRIO (CONFIG_I2C_INIT_PRIORITY - 1) SYS_INIT(mpsl_fem_init, POST_KERNEL, FEM_INIT_PRIO); Providing Kconfig option MPSL_FEM_INIT_PRIORITY with appropriate BUILD_ASSERT gives ability to have correct priority of FEM initialization in case the Kconfig I2C_INIT_PRIORITY is changed. For nrf21540_gpio, nrf21540_gpio_spi and simple_gpio the default value is equal to KERNEL_INIT_PRIORITY_DEVICE (no change in default value and behavior) is used. The SYS_INIT priority at which legacy forwarding of pins happens (mpsl_fem_host_init sys init priority) is intentionally not affected. Signed-off-by: Andrzej Kuros <andrzej.kuros@nordicsemi.no>
Add simple dect sample. Signed-off-by: Eivind Jølsgard <eivind.jolsgard@nordicsemi.no>
f07bae7
to
e7ee15c
Compare
opened to NCS |
Add simple dect sample.
TODO: