Skip to content
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

Consistent HAL and API for all microcontrollers #787

Open
buha opened this issue Oct 31, 2023 · 4 comments
Open

Consistent HAL and API for all microcontrollers #787

buha opened this issue Oct 31, 2023 · 4 comments
Assignees
Labels
API Change This issue or pull request involves a change to the current MSDK API enhancement New feature, request, or updating to latest version

Comments

@buha
Copy link
Contributor

buha commented Oct 31, 2023

Hi,

At ADI's no-OS, we use msdk to run no-OS projects on several Maxim microcontrollers.

We try to support all of them through msdk but unfortunately the API (exposed functions and data structures) of msdk is not consistent.

This has led us to a dilemma, either use a lot of #ifdefs to be able to handle all the differences, or duplicate code, and have all the code specific to max32650 in one file, all the code specific to max32655 in another file etc. We chose the 2nd one, but really, both are equally bad and the only way to fix this is to address the root cause.

You're probably aware of the differences, but here's how they look like from a user's point of view:

Screenshot from 2023-10-31 08-40-46

Screenshot from 2023-10-31 08-41-20

Screenshot from 2023-10-31 08-41-12

These are minor differences that could be eliminated in msdk, so that whoever wants to use a truly common HAL and API to be able to quickly switch between a target microcontroller and another.

@Jake-Carter
Copy link
Contributor

Hi @buha, I agree. It's been something that I've wanted to address for a while.

At the register-level, there's going to be unavoidable differences. We generate our register files from the definitions and names we get from design, and we want those to match exactly what is written in the User Guides. Design will often change the names of registers/fields between parts. If you find yourself interacting with the registers directly and an API function doesn't exist to suit your needs, I would suggest raising a ticket so that we can create one.

In general, our API should be able to abstract those differences away, and ideally the user never needs to go to the registers. As a first step, I have been in favor of consolidating our API into a single set of header files for all micros. This would make the API match across all micros and make the user interface consistent. There are a couple challenges here:

  1. The peripherals that are available vary from part to part. We will need to preserve modularity so that it's clear what modules are supported for each part.
  2. We've painted ourselves into a corner with some of the API functions. In some cases, the same top-level functions have different arguments to account for hardware differences. This is a design mistake, but now we will need to avoid breaking projects that depend on the existing function prototypes.
  3. As you've seen, our enums and data structures are inconsistent. Consolidating them will be the biggest effort, and doing it in a way that doesn't break legacy projects will be a big challenge.

We are very much open to suggestions and contributions here.

Another alternative idea that I've been exploring is adding a C++ specific version of the API that addresses these issues now that I've got C++ support working and it seems stable. It's more of a "green field" and there aren't the concerns about breaking the existing C API. C++ gets us some nice features like object instantiation and function overloading. I want to make sure that we make good choices here so this is very much in the early design phase.

... so that's where we stand currently on this. We've got a team meeting on Wednesday where I'll bring this up again. Since your team is having these challenges as well we may be able to bump up the priority on this a bit.

In the meantime, I think it would be a good approach to log the places where you're going to the register-level. Check if there's an API function that could suit your needs instead, and if not we should create one. I think this will help abstract away some complexity in the short-term. Your GPIO set_enable function above is a good example - typically you don't interact with this bit alone. It's used in conjunction with the other enable bits to set the alternate function for a pin.

In our API, MXC_GPIO_Config is the intended function to abstract the alternate function selection way.

image

@Jake-Carter Jake-Carter self-assigned this Nov 2, 2023
@Jake-Carter Jake-Carter added enhancement New feature, request, or updating to latest version API Change This issue or pull request involves a change to the current MSDK API labels Nov 2, 2023
@buha
Copy link
Contributor Author

buha commented Nov 3, 2023

Hi @Jake-Carter
And thanks for the detailed answer!

We avoid using register accesses directly and rely on API functions and data abstractions with probably just a few exceptions (I know of parts of our GPIO and SPI peripheral drivers, for example, that use register accesses for optimization reasons).

We've painted ourselves into a corner with some of the API functions. In some cases, the same top-level functions have different arguments to account for hardware differences. This is a design mistake, but now we will need to avoid breaking projects that depend on the existing function prototypes.

We are very much open to suggestions and contributions here.

How about compiler warnings for particular functions of the API being deprecated in the "next" release ?
It's really your call, but I think it's acceptable to break an API provided you notify it enough time in advance.

Here's an example that I illustrate as a 4-step process for a modification to a function that involves changing both the types and the number of arguments. As a side-effect, the function name is also changed (I guess it's not possible without function overloading of higher languages such as C++).

Step 1: current state of affairs, API provides a div() which is used by the API user in their main().

#include <stdio.h>

int div(int a, int b)
{
    return a / b;
}

void main()
{
    printf("div(5, 4) = %d\n", div(5, 4));
}

Step 2: Issue deprecation warning in a release, provide alternative API div2() and associated data types.

#include <stdio.h>

__attribute__((deprecated)) int div(int a, int b)
{
    return a / b;
}

struct fraction {
    float divider;
    float dividend;
};

float div2(struct fraction f)
{
    return f.divider / f.dividend;
}

void main()
{
    printf("div(5, 4) = %d\n", div(5, 4));
}

Step 3: API user may see the warning and adapt their main() (or not !, in which case it's acceptable, due to them probably not actively developing throughout the deprecation notification period):

#include <stdio.h>

__attribute__((deprecated)) int div(int a, int b)
{
    return a / b;
}

struct fraction {
    float divider;
    float dividend;
};

float div2(struct fraction f)
{
    return f.divider / f.dividend;
}

void main()
{
    struct fraction f = {5.0, 4.0};
    printf("div2(f) = %f\n", div2(f));
}

Step 4: Remove deprecated API in a new release

#include <stdio.h>

struct fraction {
    float divider;
    float dividend;
};

float div2(struct fraction f)
{
    return f.divider / f.dividend;
}

void main()
{
    struct fraction f = {5.0, 4.0};
    printf("div2(f) = %f\n", div2(f));
}

Maybe there are some well-thought and structured methods for doing this, this one is half-baked...

Anyway, on behalf of our team, we appreciate you're looking into this and that you plan to discuss this internally and find a solution!

@Jake-Carter
Copy link
Contributor

Thanks, yes it's a good option but we want to avoid overdoing it. It can be difficult for users to port to the new functions, especiallt as they add up over time.

I'll keep you updated. Likewise we're happy to have the no-OS support you're working on. Maybe we can integrate more directly with your repo, or vice versa. I've got the MSDK User Guide and our build system in a pretty comprehensive state now. Same for our VS Code support. We could at probably link our documentation at least

@Jake-Carter
Copy link
Contributor

Hi @buha, spoke with my manager and team about it this week and the priority for consolidating our APIs has been set low - though it is still something I'll work on when I can.

As we discussed we've just painted ourselves into a corner with some of the hardware differences between micros that were not abstracted away well originally. Avoiding breaking changes to the API is the primary issue here...

In the meantime let me know if there are any missing APIs or broken functionality. That type of work does get higher priority.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Change This issue or pull request involves a change to the current MSDK API enhancement New feature, request, or updating to latest version
Projects
None yet
Development

No branches or pull requests

2 participants