Jeetrohan mpu60xx 1 (BSP-568)

[jeetrohan]

ESP-BSP Pull Request checklist

Note: For new BSPs create a PR with this link.

• Version of modified component bumped
• CI passing

Related

• Feature Request: Support for DMP Mode in MPU6050 Driver (BSP-482) #317

Change description

A new mpu60xx library which uses the new I2C driver for reading raw and floating point values; from accelerometer, gyrometer and temperature sensor.
Added functionality to use inbuilt motion detection on mpu60xx using both, polling and interrupts.

[CLAassistant]

All committers have signed the CLA.

[tore-espressif]

@jeetrohan That's a pretty good job for your first contribution!

[jeetrohan]

I just realized the merge here is different from the fork where sync simply updates the branch. 😅

[jeetrohan]

@espzav , @tore-espressif
If I'm not wrong the pre-commit failed because of incorrect indentation, which it has fixed (or so it indicates in output).
A rerun should do it.

[tore-espressif]

@espzav , @tore-espressif If I'm not wrong the pre-commit failed because of incorrect indentation, which it has fixed (or so it indicates in output). A rerun should do it.

The pre-commit job is always run on latest changes, so there are probably still some formating errors.

If you want to fix them locally, in your esp-bsp folder you can do

pip install pre-commit && pre-commit install

To install pre-commit. Then you can run the pre-commit check on you last 11 commits like this

pre-commit run --from-ref HEAD~11 --to-ref HEAD

[tore-espressif]

@jeetrohan Thanks again for the PR, I'll take over and fix the remaining things when I'm free

[jeetrohan]

@espzav , @tore-espressif If I'm not wrong the pre-commit failed because of incorrect indentation, which it has fixed (or so it indicates in output). A rerun should do it.

The pre-commit job is always run on latest changes, so there are probably still some formating errors.

If you want to fix them locally, in your esp-bsp folder you can do

pip install pre-commit && pre-commit install

To install pre-commit. Then you can run the pre-commit check on you last 11 commits like this

pre-commit run --from-ref HEAD~11 --to-ref HEAD

@tore-espressif
Thank you.
I'll do this at the earliest. I'd have done it since the beginning but as I mentioned it was my first time using the git. So, I think I'll still need to look more into some documentation to do this properly.
I'll follow these for the next update onward too, so that merging process is not hindered by these simple problems which should be resolved at my end itself.

[jeetrohan]

@jeetrohan Thanks again for the PR, I'll take over and fix the remaining things when I'm free

@tore-espressif I totally understand that. I myself had thought to do pre-commit test on branch, but due to some other liabilities and my lack of knowledge (about git itself) I wasn't able to do it ( even though I really want to).
I'll keep you posted, once I'll add more functionality (& this time with pre-commit check. I believe I should've been the one to do it.)
Thanks again.

[jeetrohan]

hi @tore-espressif & @espzav
I know its been quite some time since I made the last commit.
There were some irl issues after then, and later I was unwilling to touch again mpu60xx because its quite the subsystem in itself.
I've made a recent commit which is pre-committed and also has the features I promised earlier.
There are still many things I wanted to add.
But I wasn't sure if this PR is still acceptable.
Please review and let me know if I need to make changes or something else.

[jeetrohan]

@tore-espressif I think I'll need to update the idf version to >=5.3 in idf_component.yml to avoid test failures (it says esp_driver_.
or
I can use i2c_bus component from component registry for backward compatibility.

Which one of the above do you suggest?

edit:
I have to apologize.
I realized that it maybe because I had revoked write permissions..
please wait for a day or two..
I needed to change a few things in this submission.

[jeetrohan]

@tore-espressif, @espzav

I have fixed the issues.
Please do a re-run.

[jeetrohan]

@tore-espressif , @espzav
from idf_component.yml in the most recent update:

dependencies:
  idf: ">=5.2"

from the error log :

.Using component placed at /__w/esp-bsp/esp-bsp/components/mpu60xx for dependency mpu60xx(*), specified in /__w/esp-bsp/esp-bsp/components/mpu60xx/examples/mpu_motion_detect/main/idf_component.yml
.HINT: Please check manifest file of the following component(s): main
CMake Error at /opt/esp/idf/tools/cmake/build.cmake:548 (message):
  WARNING: Component "idf" not found

  WARNING: Component "idf" not found

  WARNING: Component "idf" not found

  WARNING: Component "idf" not found

  ERROR: Because project depends on idf (>=5.4.0) which doesn't match any
  versions, version solving failed.

And also the the above log is from build (release-v5.1, 2, 1)
I am not sure if the run should be performed on idf<5.2.

If I'm wrong let me know.
thanks.

[tore-espressif]

@jeetrohan I'm sorry for the late responses, I'll do my best to have a look by the end of this week

[jeetrohan]

@jeetrohan I'm sorry for the late responses, I'll do my best to have a look by the end of this week

@tore-espressif
No worries at all, thank you for the update! I appreciate you taking the time to look into it, and I’m happy to wait until the end of the week. Let me know if you need anything from me in the meantime.

[tore-espressif]

@jeetrohan thanks for your patience and for all the hard work you’ve put into these updates

Your implementation of the advanced MPU60xx features is very valuable for projects that need that extra functionality. At the same time, the goal of esp-bsp is to stay focused on a core set of drivers and examples that we can reliably maintain over time. Because the MPU60xx enhancements extend beyond what we normally cover in esp-bsp, we worry that adding them here could make it harder to keep everything tested and up to date.

To make sure your work gets the visibility it deserves—and so that you can continue to iterate on those advanced features without constraints—it would be great to publish this as its own component in the ESP Registry under your GitHub namespace. That way:

1. You stay in full control of the code.
   You’ll be free to add new features, squash bugs, or make breaking changes as needed, without impacting the esp-bsp maintainers’ stability guarantees.

2. Users can still pull it in very easily.
   Anyone using IDF Component Manager can add your repo to their prject and get those MPU60xx enhancements right away.

3. esp-bsp remains lean and maintainable.
   We can continue focusing on the drivers and examples that serve as a stable, well-tested foundation, while your component lives and evolves alongside it.

The process to register a component in the ESP Registry is covered in the IDF Component Manager docs. You can sign in with your GitHub account, and within a few minutes have it listed. Once it’s published, folks who rely on esp-bsp can still pull in your MPU60xx component as an additional dependency.

I really appreciate your contribution here—this is exactly the kind of work we want to encourage—and I think publishing it as a standalone, registry-managed component is the best path forward so both projects can thrive. Let me know if you need any help with the IDF Component Manager setup!

[jeetrohan]

@tore-espressif
Thankyou for your kind words. I'll be closing this PR with this final comment because I believe if there was a way to merge you would have suggested that, not the reasons you have given because
This PR was refactored specifically to make maintenance of this driver possible.
And if I wanted to stay in control I would not have submitted a PR to a official project.

Assuming

MPU60xx enhancements extend beyond what we normally cover in esp-bsp

I'd request you or someone on your team to review :
refactor(bh1750): migrate to esp_i2c_driver, add example and update test app (BSP-681)

It is a simple refactor as it says and has been pending for over a week. Maybe I can remove the example I added, based off the test app.

And the Component Registry
Please do not take the following personally, it is not directed at you but a simple observation:
The components registered in there are used only if they are from Espressif namespace.
I've made a submission once and probably never will again.

PS:
I would have rather preferred an honest response, older boards are now obsolete and we have moved towards supporting the new ones which do not require these sensors (MPU6050, BH1750, etc..), anymore.
These are here just for backward compatibility now.
Probably you didn't say this because you are not allowed to divulge the future policies, and I can understand that too.

[tore-espressif]

I am allowed to say whatever apart from insulting people :D

The more straightforward answer is: We cannot justify spending time reviewing >1500 line of code from external contributors.

[jeetrohan]

I am allowed to say whatever apart from insulting people :D

The more straightforward answer is: We cannot justify spending time reviewing >1500 line of code from external contributors.
@tore-espressif
And you could have said this, from the start, particularly when I made the first PR.
I rather appreciate honesty than spending time on something meaningless.