Add Objects Extensions Configuration Parameter to HAL

[AGlass0fMilk]

Summary of changes

I am proposing a new method of extending Mbed with external application code and libraries. I thik my use case better explains the need for this kind of extension hook:

I am developing a driver for a relatively new chip from TI, the TCAN4551. It's a very cost effective way to add CAN 2.0a/b and CAN-FD to one's system. It provides both a CAN transceiver and controller that an MCU can interact with over SPI.

From an abstraction perspective, it would be great to be able to use Mbed's existing CAN API so that any examples or existing code will work on platforms that import this driver without any other steps.

Right now there isn't a clear way to add hardware drivers like this outside of Mbed's main source. I can't enable DEVICE_CAN without causing missing symbols issues during the build because my target (in this case nRF52840) does not define hal structures for the CAN HAL API.

My proposed solution to allow this kind of extension is as follows:

An external library or application can provide an objects_extensions_xxx.h file (eg: objects_extensions_can.h). The mbed_lib.json of that library can add the appropriate target.device_has_add flags to the configuration (eg: CAN).

The application is then responsible for:
1.) Enabling hal.enable-objects-extensions by setting this configuration to true in mbed_app.json
2.) Providing an overall objects_extensions.h header that subsequently imports each individual library's objects_extensions_xxx.h header. This allows multiple extensions libraries to be used if the application calls for it without requiring the application developer to change file names and such to avoid conflicts.

Impact of changes

Migration actions required

Documentation

---

Pull request type

[X] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

---

Test results

[] No Tests required for this change (E.g docs only update)
[X] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

---

Reviewers

---

[ciarmcom]

@AGlass0fMilk, thank you for your changes.
@ARMmbed/mbed-os-hal @ARMmbed/mbed-os-maintainers please review.

[bulislaw]

Hm I really like the idea of external drivers using Mbed OS APIs when the provide the same functionality, but I don't really like the implementation proposed here. I would imagine that if external driver provides all the necessary blocks it shouldn't need any special flags, especially if they are used to modify the HAL implementation for each board. I'd rather make some deeper changes in how the drivers and HAL implementation are composed and work.
Can you share some more details on why we need to change object.h for each board?

[AGlass0fMilk]

Hm I really like the idea of external drivers using Mbed OS APIs when the provide the same functionality, but I don't really like the implementation proposed here.

Yeah, I'm not a huge fan of it either that's why I opened this PR. It's easier to get ideas across and start a conversation when there's already something done.

I would imagine that if external driver provides all the necessary blocks it shouldn't need any special flags, especially if they are used to modify the HAL implementation for each board. I'd rather make some deeper changes in how the drivers and HAL implementation are composed and work.
Can you share some more details on why we need to change object.h for each board?

You may already have a solid understanding of all this architecture, but for the sake of clarity (especially to other reading this) I will go over why I chose the current approach:

So as it stands with the current architecture, the target's HAL implementation must define structures that facilitate usage of the target-specific drivers. i.e: The control blocks and information used for a ST micro SPI driver are likely going to be different from those needed by an NXP SPI driver.

This flexibility is implemented in the following way:

A HAL API header includes a target's device.h, as shown here in can_api.h:

mbed-os/hal/can_api.h

Lines 23 to 30 in 69a7d67

	#include "device.h"
	#include "pinmap.h"
	#if DEVICE_CAN
	#include "PinNames.h"
	#include "PeripheralNames.h"
	#include "hal/can_helper.h"

device.h seems to be a legacy header that used to handle macros that are now defined by the build system. In some targets it allows implementers to add device-level macros. device.h also subsequently includes objects.h:

mbed-os/targets/TARGET_NORDIC/TARGET_NRF5x/TARGET_NRF52/TARGET_MCU_NRF52840/TARGET_NRF52840_DK/device.h

Lines 35 to 37 in 69a7d67

	#include "objects.h"

objects.h is where all the target-specific structures are defined. For example, the CAN HAL API passes around a can_t instance to all HAL function calls to differentiate between different CAN devices in a target (in case there are multiple of the same peripheral). The can_t struct is nothing more than a renamed struct, can_s, that is defined by the target's objects.h:

mbed-os/hal/can_api.h

Line 68 in 69a7d67

typedef struct can_s can_t;

mbed-os/targets/TARGET_STM/TARGET_STM32F4/common_objects.h

Lines 141 to 147 in 69a7d67

	#if DEVICE_CAN
	struct can_s {
	CAN_HandleTypeDef CanHandle;
	int index;
	int hz;
	};
	#endif

(In this case, it's in common_objects.h due to how ST's drivers are shared between different targets, but ultimately it's included by objects.h for the specific target)

---

Now, since I am trying to make a driver that can extend a target to "have" a CAN peripheral, I need a way to inject my custom control structures into the objects.h for a given target. This way, my custom objects.h is included by the HAL and everything builds as if the target has a built-in CAN controller peripheral and everyone's happy.

I currently see no way to cleanly add this kind of extension outside of just including an extra header conditionally based on some build flag. So that's where we're at.

I'm open to suggestions. This is definitely out of the ordinary use case but it makes Mbed more extensible in the long run.

[0Grit]

@AGlass0fMilk I had conversations about external IC drivers in other issues in this repository.
We have had similar conversations internally.

I'd love to actually review the PR and converse further but considering my current situation I don't really have time.

[AGlass0fMilk]

@bulislaw Any additional thoughts on this? You can see my use case here:

https://github.com/AGlass0fMilk/mbed-tcan4551

That extension library adds CAN to any Mbed target that doesn't have CAN already (but has SPI).

It's working in beta-status I'd say. I still have to update it to support use by targets that already have CAN but want more CAN interfaces.

[AGlass0fMilk]

Associated board for anyone who stumbles upon this later:

[adbridge]

@ARMmbed/mbed-os-hal @bulislaw any comments / review on this ?

[bulislaw]

Hi @AGlass0fMilk I'm really sorry I dropped the ball here. I'm afraid myself and most of the people here are flat out. Thanks for the contribution, I think it's very good proposal and something we need to work on, but I'd like to solve it in more systematic way.
Here we introduce a new global HAL flag that only works for one target and even if we would to copy it to all the device.h files it would mean that each new target would need to remember to include this conditional include. That's not ideal.
I'm really sorry, but I really can't spend any meaningful time on this. Would it unblock you if we would make it specific to this target for now? I mean instead adding the flag in HAL's mbed_lib.json do it in targets.json just for this one target?

There's some design work progressing on unifying the pin naming across the OS and targets lead by @malavikasajikumar. I think we should consider these two together and try to look at unifying the pin names, defining peripherals and using existing drivers for any external peripherals. That's also quite important in context of custom hardware that may integrate SoC with extra peripherals.

[AGlass0fMilk]

Hi @bulislaw no problem, I just started working on this project again recently.

It would work for me to attach this to a couple targets (nRF2840_DK/EP_AGORA).

Can this "feature request" be added to the list of design considerations when the build system/architecture is reworked?

In the meantime, I'll update this PR to minimize the changes

Pull request has been modified.

[mergify]

This PR cannot be merged due to conflicts. Please rebase to resolve them.

[AGlass0fMilk]

Updated PR to only apply changes to NRF52840_DK and EP_AGORA until the architecture is refactored to allow this kind of extension in a more robust way.

[AGlass0fMilk]

Rebased, @0xc0170 please review.

[0xc0170]

So after the rework the scope of this PR is just to add extensions in for 2 targets. Where we can find this extensions being active in place?

[AGlass0fMilk]

So after the rework the scope of this PR is just to add extensions in for 2 targets. Where we can find this extensions being active in place?

It is used in conjunction with this library here:

https://github.com/AGlass0fMilk/mbed-tcan4551

To add Mbed-OS CAN functionality to targets that do not already have it.

I can push up an example if you need?

[0xc0170]

I can push up an example if you need?

I checked, that is what I wanted to see.

LGTM as it is here now.

[0xc0170]

CI restarted

[mbed-ci]

Test run: SUCCESS

Summary: 6 of 6 test jobs passed
Build number : 1
Build artifacts

[0xc0170]

I updated the description - this is now just patch update (so do not need any migration/impact).