ESL PR separate into subsys and sample #12012

pirun · 2023-08-11T03:37:26Z

Separate ESL application into two parts.

BLE ESL service and profile in subsys/bluetooth/service.
ESL AP and Tag sample in samples/bluetooth.

NordicBuilder · 2023-08-11T03:38:41Z

Test specification

CI/Jenkins/NRF

Integration Platforms

CI/Jenkins/integration

Test Module	File based changes	Manually selected	West overwrite
desktop52_verification	X
test-fw-nrfconnect-ble	X
test-fw-nrfconnect-ble_samples	X
test-sdk-find-my	X

Detailed information of selected test modules

Note: This message is automatically posted and updated by the CI

NordicBuilder · 2023-08-11T04:22:42Z

You can find the documentation preview for this PR at this link. It will be updated about 10 minutes after the documentation build succeeds.

Note: This comment is automatically posted by the Documentation Publishing GitHub Action.

alwa-nordic · 2023-08-15T13:03:17Z

There is a compliance failure. It's rejecting the images as binary files. Do you have a solution for this?

alwa-nordic · 2023-08-15T13:05:00Z

Does this PR replace the old PR? If so can you close the old one?

pirun · 2023-08-16T02:11:01Z

Does this PR replace the old PR? If so can you close the old one?

This is a draft to show how ESL application PR could be separated into smaller chunk PR.
Will be closed soon as well as old PR

pirun · 2023-08-24T04:13:33Z

Re-open for tech writer document review.

greg-fer · 2023-08-30T12:02:31Z

Adding @annakielar and @peknis to take care of the docs, as per mail thread.

doc/nrf/libraries/bluetooth_services/services/esl.rst

samples/bluetooth/central_esl/README.rst

samples/bluetooth/central_esl/central_esl_subcommands.rst

Add electronic shelf label service (ESLS). Add electronic shelf label service (ESLS) client. It implemented all mandatory features. Leave all hardware-related control to callback functions. Signed-off-by: Pirun Lee <pirun.lee@nordicsemi.no>

Add a sample running electronic shelf label service as tag role. The sample use LED on DKs to simulate EPD display. This sample provides real EPD reference code as well. Add a sample running esl client as AP role. Signed-off-by: Pirun Lee <pirun.lee@nordicsemi.no>

pirun · 2023-09-06T14:07:09Z

Change PR status to trigger doc build.

Add esl service and profile library document. Signed-off-by: Pirun Lee <pirun.lee@nordicsemi.no>

Tuning the library docs to match them to the template, at least to some extent. Signed-off-by: Pekka Niskanen <pekka.niskanen@nordicsemi.no>

A few more minor fixes to the docs, based on answers from the SME. Signed-off-by: Pekka Niskanen <pekka.niskanen@nordicsemi.no>

Using a link to the sample doc. Signed-off-by: Pekka Niskanen <pekka.niskanen@nordicsemi.no>

maje-emb · 2023-10-02T11:10:01Z

doc/nrf/libraries/bluetooth_services/services/esl.rst

-You need to implement the following callbacks to if display device exists on ESL Tag.
-The :c:func:`display_init` callback is used to initialize the display device if additional work is not done by display driver. e.g.: Initialize memory region for framebuffer, set font type. To implement this callback, you need to write a function that initializes the display device.
+  This function is called when the tag device is booting up and unassociated.
+  To implement this callback, you need to write a function that takes the index of the display device and displays the necessary information on the device.


Do we need to describe what parameters the function takes?
This is described in the API documentation section.

Similarly in other places.

maje-emb · 2023-10-02T11:44:54Z

doc/nrf/libraries/bluetooth_services/services/esl.rst

+  If the sensor reading is successful, value ``0`` is returned.
+  If the sensor reading is not fast enough or fails, value `-EBUSY` is returned.


A detailde description of the returned values by the funkction shoud be in the API dokumentation.
There is a sentence there: "If the operation was successful. Otherwise, a (negative) error code is returned", this can be extended.

maje-emb · 2023-10-18T14:24:18Z

samples/bluetooth/peripheral_esl/README.rst

+Power Consumption Measurement
+=============================
+
+Prerequisite: Read :ref:`app_power_opt_general` and get `Power Profiler Kit II (PPK2)`_
+
+1. Build and program the Tag (:ref:`nrf52833dk_nrf52833`) Power Profiling configuration by following :ref:`peripheral_esl_build_and_program` and :ref:`peripheral_esl_power_profiling_variant`
+
+#. Follow `Preparing a DK for current measurement`_ to prepare 52833DK for Power Consumption measurements
+
+   a. Cut SB40.
+   b. Connect VOUT of PPK2 to nRF current Measurement on DK, connect GND of PPK2 to External supply negative polarity pin (`-`). By doing so, PPK2 will provide power to only the nRF52 SoC.
+   c. Connect the PPK2 (powered ON) in “Source Meter” mode to the :ref:`nrf52833dk_nrf52833` (turned ON) and enable power output 3000mV on the Power Profiler GUI and start measurement. Power the rest of the ESL Tag DevKit via USB (same as programming USB port), and keep the DevKit turned ON.


It seems to me that in the sample dokumentation we don't need a dedscription of how to use the Power Profiler Kit. This is included in the PPK documentation.

maje-emb

Some general suggestion from me.
ESL Service:

UUID definitions for this service should be in the zephyr/include/zephyr/bluetooth/uuuid.h file.
It seems that the service can be registered statically. We don't need to use "Bluetooth GATT attribute pools"
Initialization of the display, LEDs, sensors should be outside the service.
The setup of advertising, PAwR should be outside the service.
The OTS should run alongside the ESL service; it is not a part of it.
There are a lot of work_q and work Items used. Are all of them necessary? I'm worried that it could lead to a race condition. Please optimize it.
Connection and bond management should be outside the service.

ESL Client:

Scanning and advertising should be outside.
The discovery process should be outside.
Client should be able to read or write all the service characteristics.

Other:

Are the Kconfig.esl_client.defaults and Kconfig.esl.defaults files necessary?
In file names, functions, etc., consider using the acronym "ESLS" (Electronic Shelf Labels Service).
In the /nrf/include/bluetooth/services/ directory, I suggest that there should be only esl.h and esl_client.h files.

The proposal for changes to the Service and Client APIs will be discussed offline.

greg-fer · 2023-11-02T09:10:18Z

I might've been wrong in my assessment and the ESL might fall into the category of demos according to https://nordicsemi.atlassian.net/wiki/spaces/NC/pages/56004256/Product+requirements . @peknis , @annakielar , @pirun , could you discuss this with Lorenzo and Eirik?

github-actions · 2024-10-18T00:32:37Z

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

pirun requested review from alwa-nordic, jori-nordic, carlescufi and KAGA164 as code owners August 11, 2023 03:37

github-actions bot added the doc-required PR must not be merged without tech writer approval. label Aug 11, 2023

pirun marked this pull request as draft August 11, 2023 03:38

pirun added the DNM label Aug 11, 2023

pirun closed this Aug 16, 2023

pirun reopened this Aug 24, 2023

pirun force-pushed the esl_pr_seperate_sample_and_subsys branch from 46a6c88 to 85538a3 Compare August 24, 2023 04:14

greg-fer requested review from peknis and annakielar August 30, 2023 12:02

peknis requested changes Sep 4, 2023

View reviewed changes

pirun force-pushed the esl_pr_seperate_sample_and_subsys branch from 85538a3 to 0bcfdb0 Compare September 6, 2023 06:32

pirun added 2 commits September 6, 2023 22:04

pirun marked this pull request as ready for review September 6, 2023 14:07

pirun removed the DNM label Sep 6, 2023

pirun force-pushed the esl_pr_seperate_sample_and_subsys branch from da36dc0 to 1844c74 Compare September 6, 2023 14:13

Doc: Add esl service and profile document

2d630c1

Add esl service and profile library document. Signed-off-by: Pirun Lee <pirun.lee@nordicsemi.no>

pirun force-pushed the esl_pr_seperate_sample_and_subsys branch from 1844c74 to 2d630c1 Compare September 6, 2023 15:48

pirun marked this pull request as draft September 7, 2023 00:02

pirun added the DNM label Sep 7, 2023

peknis added 3 commits September 20, 2023 10:33

doc: updates to ESL library docs

4157c8e

Tuning the library docs to match them to the template, at least to some extent. Signed-off-by: Pekka Niskanen <pekka.niskanen@nordicsemi.no>

doc: library doc updates

29d92a6

A few more minor fixes to the docs, based on answers from the SME. Signed-off-by: Pekka Niskanen <pekka.niskanen@nordicsemi.no>

doc: yet another update to ESLS client lib doc

288c5fb

Using a link to the sample doc. Signed-off-by: Pekka Niskanen <pekka.niskanen@nordicsemi.no>

maje-emb reviewed Oct 3, 2023

View reviewed changes

maje-emb reviewed Oct 18, 2023

View reviewed changes

maje-emb reviewed Nov 2, 2023

View reviewed changes

jori-nordic removed the request for review from annakielar December 12, 2023 07:02

github-actions bot added the Stale label Oct 18, 2024

github-actions bot closed this Nov 7, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ESL PR separate into subsys and sample #12012

ESL PR separate into subsys and sample #12012

pirun commented Aug 11, 2023

NordicBuilder commented Aug 11, 2023 •

edited

Loading

NordicBuilder commented Aug 11, 2023

alwa-nordic commented Aug 15, 2023

alwa-nordic commented Aug 15, 2023

pirun commented Aug 16, 2023

pirun commented Aug 24, 2023

greg-fer commented Aug 30, 2023

pirun commented Sep 6, 2023

maje-emb Oct 2, 2023

maje-emb Oct 2, 2023

maje-emb Oct 18, 2023

maje-emb left a comment •

edited

Loading

greg-fer commented Nov 2, 2023

github-actions bot commented Oct 18, 2024

		If the sensor reading is successful, value ``0`` is returned.
		If the sensor reading is not fast enough or fails, value `-EBUSY` is returned.

ESL PR separate into subsys and sample #12012

ESL PR separate into subsys and sample #12012

Conversation

pirun commented Aug 11, 2023

NordicBuilder commented Aug 11, 2023 • edited Loading

Test specification

CI/Jenkins/NRF

CI/Jenkins/integration

NordicBuilder commented Aug 11, 2023

alwa-nordic commented Aug 15, 2023

alwa-nordic commented Aug 15, 2023

pirun commented Aug 16, 2023

pirun commented Aug 24, 2023

greg-fer commented Aug 30, 2023

pirun commented Sep 6, 2023

maje-emb Oct 2, 2023

Choose a reason for hiding this comment

maje-emb Oct 2, 2023

Choose a reason for hiding this comment

maje-emb Oct 18, 2023

Choose a reason for hiding this comment

maje-emb left a comment • edited Loading

Choose a reason for hiding this comment

greg-fer commented Nov 2, 2023

github-actions bot commented Oct 18, 2024

NordicBuilder commented Aug 11, 2023 •

edited

Loading

maje-emb left a comment •

edited

Loading