Location: Add header-only recipes#2277
Conversation
Add initial header-only recipes for location module Signed-off-by: Vamana Murthi <vthuniki@qti.qualcomm.com>
lumag
left a comment
There was a problem hiding this comment.
- It's
recipes-navigationrather thanrecipes-location. - What are the users for these headers?
| # Common bitbake recipe information for location meta layer. | ||
| # Below are common values, statements and functions. | ||
|
|
||
| PACKAGE_ARCH ?= "${TUNE_PKGARCH}" |
There was a problem hiding this comment.
This is definitely not worth having a separate include. Fold it in annd provide an explanation, why it is required. BTW, why?
There was a problem hiding this comment.
why?
This seem redundant, I will fix it.
From my initial experiments, PACKAGE_ARCH was influencing the naming of the generated tar files for proprietary modules. I was informed that the tar artifacts needed to be aligned with the armv8 architecture. Hence i explicitly set PACKAGE_ARCH to ${TUNE_PKGARCH}.
However, after further validation, I confirmed that when not explicitly set, the default PACKAGE_ARCH already resolves to the appropriate armv8 value. So this assignment is redundant, and I’ll remove it in the next update.
definitely not worth having a separate include
My baseline code is common between Android and LE.
In my follow-on PRs, there are recipes where we need to define a few CFLAGS that are used in the source code to differentiate parts of the code between Android and LE, and to enable some inlines/macros specific to the LE baseline. For that, we are using this .inc file to define those configurations.
example:
EXTRA_OECONF = " --with-glib"
code ref:
https://git.codelinaro.org/clo/le/platform/hardware/qcom/gps/-/blob/location.lnx.0.0/utils/log_util.h?ref_type=heads#L78
Without the GLIB macro defined, I ran into an undefined macro error for ALOGE.
# compiler flag to identify QClinux target
CFLAGS:append = " -DLOC_QCLINUX_TARGET "
CPPFLAGS:append = " -DLOC_QCLINUX_TARGET "
There are some code compile decisions based on these CFLAGS.
https://git.codelinaro.org/clo/le/platform/vendor/qcom-opensource/location/-/blob/location.lnx.8.5.r1-rel/client_api_testapp/main.cpp?ref_type=heads#L2655
Hence, I wanted to retain this file in the initial phase.
There was a problem hiding this comment.
No. Add that file when actually necessary rather than when thinking that it will be necessary in future (which might never come). Also be sure that you correctly split flags between CFLAGS and CPPFLAGS. Having the same flag in both is a definite mistake. CFLAGS should be used for the flags specific to the C compiler. All preprocessor options (-I, -D, etc.) should go to CPPFLAGS.
| @@ -0,0 +1,3 @@ | |||
| # include for recipe files which require BSD-3-Clause license | |||
| LICENSE = "BSD-3-Clause-Clear" | |||
| LIC_FILES_CHKSUM = "file://${COMMON_LICENSE_DIR}/BSD-3-Clause-Clear;md5=7a434440b651f4a472ca93716d01033a" | |||
There was a problem hiding this comment.
NAK. Inline it and checksum the actual license file in the source tree.
There was a problem hiding this comment.
I checked how this is handled in other modules (e.g., recipes-multimedia/camx/camx-dlkm) and will update this accordingly.
My only concern is that github repository is currently undergoing LOST scanning, and it may take some time before the repo is created and license file can be added to the source tree.
In the meantime, I would appreciate your guidance on how you would prefer me to proceed, so that the follow-on recipes can continue to be reviewed without delay.
Rewrite SRC_URI to inline format to align with Yocto style guidelines and improve readability Signed-off-by: Vamana Murthi <vthuniki@qti.qualcomm.com>
Test Results 103 files ±0 634 suites +2 6h 56m 45s ⏱️ + 8m 5s For more details on these failures, see this check. Results for commit 1a34110. ± Comparison against base commit c66b499. |
| require include/common-location-defines.inc | ||
| require include/location-license-bsd3-clause.inc | ||
|
|
||
| DESCRIPTION = "GPS Loc Platform Library Abstraction" |
There was a problem hiding this comment.
Please provide a proper SUMMARY and DESCRIPTION in both recipes.
| DESCRIPTION = "GPS Loc Platform Library Abstraction" | ||
|
|
||
| SRC_URI = "git://git.codelinaro.org/clo/le/platform/hardware/qcom/gps.git;protocol=https;branch=location.lnx.0.0;destsuffix=hardware/qcom/gps" | ||
| SRCREV = "18cec4cd877f758292f5252d47c755b50f2838b3" |
There was a problem hiding this comment.
I had taken these recipes as base from QLI 1.0. If the suggestion is to avoid revision based repo clone and always work on tip, we are okay.
Please help confirm if we need to always work on tip of the repo?
There was a problem hiding this comment.
The suggestion is to have tags, organizing the repo contents and repo changes.
|
|
||
| DESCRIPTION = "GPS Loc Platform Library Abstraction" | ||
|
|
||
| SRC_URI = "git://git.codelinaro.org/clo/le/platform/hardware/qcom/gps.git;protocol=https;branch=location.lnx.0.0;destsuffix=hardware/qcom/gps" |
There was a problem hiding this comment.
Why do we need to clone to destsuffix here if it's a separate recipe?
There was a problem hiding this comment.
This was added mainly for readability and to follow a consistent convention. If you prefer to avoid it, I can remove it. Alternatively, can i take this up as part of a separate cleanup activity as all of our recipes have the similiar pattern
There was a problem hiding this comment.
Please avoid the destdir unless required.
|
|
||
| DESCRIPTION = "location api interface" | ||
|
|
||
| SRC_URI = "git://git.codelinaro.org/clo/le/platform/hardware/qcom/gps.git;protocol=https;branch=location.lnx.0.0;destsuffix=hardware/qcom/gps" |
There was a problem hiding this comment.
Why do we need two recipes for packing headers for this same project? Why do we need to manually copy them?
There was a problem hiding this comment.
Reason for having separate recipe files comes from the Android build system, which was used as a reference in our legacy code. The same approach has been carried forward by the team into LE as well.
I agree that we can consolidate some of these and reduce the number of recipe files overall. However, I would prefer to take that up as a follow-up improvement/optimization.
For now, the goal is to enable the baseline with minimal changes to the existing recipe structure.
There was a problem hiding this comment.
prefer to take that up as a follow-up improvement/optimization.
Why? Please make it good from the beginning.
There was a problem hiding this comment.
Sure, we are looking at the options to reduce the number of recipes which are header only and combine them into a single recipe. I will get back with proposal
|
The major question from my side would be, why are we bring the whole HAL from Android? Can't we use a native implementation in style of https://github.com/qualcomm-linux/meta-qcom/blob/master/dynamic-layers/openembedded-layer/recipes-navigation/gpsd/gpsd/0001-Introduce-Qualcomm-PDS-service-support.patch ? |
The HAL repository is a common codebase shared between Android and LE, and we have maintained it this way to maximize code reuse across different software platforms.
The referenced approach appears to use the QRTR module directly. However, we have certain proprietary features that depend on shared modules from the HAL. These features cannot be supported purely using QRTR- or QMI-based implementations alone. |
Features, such as...? How are they controlled / implemented? Can they be supported separately? Why do they depend on HAL modules? |
GPSd is part of recipes-navigation.
These are some of the high-level features supported through the LCA SDK:
^these features are not part of the current POR. However, we do receive PdM/customer requests to enable them, and we plan to bring in only the required modules accordingly. Although both GPSd(with 0001-Introduce-Qualcomm-PDS-service-support.patch) and LCA use QMI interface, our exisiting customers currently rely on the LCA SDK. Therefore, we are prioritizing LCA enablement in QLI 2.0. Support for applicable features through GPSd is also planned, but will be handled as a separate effort. |
It's still navigation. As such, use recipes-navigation subdir.
It's implemented in other software tools too.
Gpsd provides broadcasting of the NMEA data to multiple clients, if that's what you mean here.
Let's schedule a meeting, discussing possible development plans and pitfalls. |
Add initial header-only recipes (loc-pla-hdr and location-api-iface) for the location module.
These are added first so that subsequent recipes can include them as dependencies.