Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Possible API Break Versus Convention and Correctness #16

Closed
jonasdn opened this issue Sep 9, 2021 · 2 comments
Closed

Possible API Break Versus Convention and Correctness #16

jonasdn opened this issue Sep 9, 2021 · 2 comments

Comments

@jonasdn
Copy link

jonasdn commented Sep 9, 2021

The Crazyflie firmware is the result of a lot of engineering (Making It Work) effort, the first commit in the GitHub repository is from 2013.

There are issues in this repository like bitcraze/crazyflie-firmware#537 (Option to use SI units in controller/power-distribution interface) and bitcraze/crazyflie-firmware#396 (Fix sign inconsistencies in pose) that points at places where we lag behind in correctness or convention. A lot of work has gone into creating and investigating these issues, perhaps mainly from @whoenig and @jpreiss, and we are grateful for it.

We would like to fix these, but, the problem is that we have APIs to consider. Like the parameter- and logging API which today has ideas about which way the coordniate system points or what unit a value is represented as. This also affects the Commander Framework / API as this API also has opinions regarding what way X, Y or Z grows in relation to reality.

CF - API

If we were to implement solutions to the issues we need to take these API into consideration. There are applications, demos, scripts and research code out there that rely on these APIs to behave a certain way. And if we change which unit a parameter- or logging variable reports in we will, perhaps silently, break those programs.

So what can we do? What are our options? I have tried to enumerate them below.

1. Go Ahead And Change Units and Coordinates - But Do Not Update APIs

We fix as much as we can of the unit, correctness and conventions bugs in the firmware, but we leave the APIs as they are. This means that we keep the name of the parameter- and logging variables and we keep the name of all Python classes and functions. This is the less amount of work, it only needs updates to the crazyflie-firmware repository.

All examples and programs will continue to run. But they might now operate under false pretenses, perhaps with no error messages. The programs might stop working because of the implicit change of API.

We will have to communicate loud and clear that this will happen beforehand.

2. Go Ahead And Change Units and Coordinates - And Update the APIs

We fix as much as we can of the unit, correctness and conventions bugs in the firmware, and we also rework our APIs! We will gather learnings from our years with the current CRTP and Python API and we update them to be more developer ergonomic as well as update the parameter- and logging variables to be more consistent in naming and SI units.

This will require quite a bit of work, both in the crazyflie-firmware repository and possibly in the the crazyflie-lib-python repository.

A big benefit of this would be that we are then open to change where the boundaries for our API lies, what we provide as libraries and what is applications responsibility. And we can address pain points that are deep and structural to todays APIs.

This will break all existing programs out there, they will no longer run. They will get error messages and will need to be updated to the new APIs. We might also get weird issues and scenarios where old library code try to interface with newer firmware. We could create policys on for how long we maintain the old API and try to have them side-by-side for a bit.

3. Go Ahead And Change Units and Coordinates - Keep Old API Forever and Add a New One

We fix as much as we can of the unit, correctness and conventions bugs in the firmware, and we also rework our APIs! We will gather learnings from our years with the current CRTP and Python API and we update them to be more developer ergonomic as well as update the parameter- and logging variables to be more consistent in naming and SI units. And! We will also keep the old API as a legacy API.

It is unclear if this is possible. We could in theory keep all of the user-facing Python API and attempt to translate between the old and new one in Python code. This might take a lot of work and effort todo, but it might be possible.

But. Fixing all the correctness, convention and units in the firmware might make it very difficult to support both APIs. It might also limit how creative and how different a new API could be, while still being translatable to the old API.

We could also investigate if it would be possible to keep the API between the firmware and the libraries (CRTP and paramter- and logging variables) but translate to be more correct in regards to Units and Conventions in our library.

Even if this was deemed possible it would not help anyone developing directly against the firmware, without using a Bitcraze library.

4. Do Not Change Units and Coordinates - Keep Old API

Accept that the crazyflie-firmware will not reach correctness in respect to units and conventions and close the issues above. We can make sure all new features going in respect SI units and convention, but consider what we have as a legacy. This is by far the one that requires the less work. zero. But it will keep the pain points and inconsistencies of the Crazyflie platform.

Conclusions

There are trade offs between all solutions. We need to decide what our current APIs are worth to us. And how painful it would be for our users to handle an API break.

Maintaining multiple APIs might be to big of an burden for both us and our code base though. This is very very tricky.

@jonasdn jonasdn changed the title Possible API Break Versus Unit, Coordinate and Convention Correctness Possible API Break Versus Convention and Correctness Sep 9, 2021
@whoenig
Copy link

whoenig commented Sep 14, 2021

I prefer the common mix of option 2 and 3: Fix the issues, add a new corrected API (with a different name), deprecate old API. The old API can stay around for a while (perhaps 6-12 months) and then be removed. We did a similar thing for the V2 Log/Param API to support more than 255 entries (there is a deprecation warning for the old API, but it has not been removed yet).

For the two referenced issues, it should be actually pretty easy. bitcraze/crazyflie-firmware#537 is only within the firmware and does not require any API changes. bitcraze/crazyflie-firmware#396 only affects the setpoint API (where we could just add a new corrected 'generic' setpoint) and logging variables (where we can just add a new variable name or group). Much harder would be the CRTP APIs, but I think those don't really require changes at this point.

@jonasdn
Copy link
Author

jonasdn commented Nov 22, 2021

So we (@bitcraze/bitcraze) are pretty much in agreement with @whoenig.

In general we prefer to solve these types of situations with deprecating and removing old interfaces and create new ones. We never want to re-purpose an old interface for something new. And in switching up SI units in the firmware we probably want to do the same for the interfaces that reach python users (values of logging variables / parameters) and that might mean deprecating and removing certain variables and parameters and adding new ones with SI units.

@knmcguire knmcguire transferred this issue from bitcraze/crazyflie-firmware May 17, 2022
@bitcraze bitcraze locked and limited conversation to collaborators May 17, 2022
@knmcguire knmcguire converted this issue into discussion #17 May 17, 2022

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants