-
Notifications
You must be signed in to change notification settings - Fork 783
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
AS5048A encoder support (AEGHB-725) #382
Conversation
demianzenkov
commented
Jul 5, 2024
- Support for AS5048A (14-bit, SPI) encoder added
👋 Hello demianzenkov, we appreciate your contribution to this project! Click to see more instructions ...
Review and merge process you can expect ...
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello, thank you for adding support for AS5048A to esp_simplefoc. There are also some code styles to pay attention to. You can use format.sh to standardize the code format after importing the idf environment.
In addition, for easy testing, please add TEST_CASE in test_esp_simplefoc.
TEST_CASE added |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, there are still some issues to be resolved, mainly including that you need to add the added "as5048a.cpp" to the CMakeLists.txt of the component, so as to ensure the normal operation of the component. In addition, can you provide the TEST_CASE log when as5048a is running normally? Mainly including the changes in angles.
There is an angle log in the test case: |
There is no need to add logs to the |
The log looks like this:
Should I add add it anywhere? |
It looks like everything is working fine, no need to add logs elsewhere. |
|
||
set(INC_FILES "port/esp/include/" "port/angle_sensor") | ||
set(IGNORE_FILES "port/esp/esp_hal_adc.cpp" "port/esp/esp_hal_mcpwm.cpp" | ||
"port/esp/esp_hal_bldc_3pwm.cpp" "port/angle_sensor/mt6701.cpp") | ||
"port/esp/esp_hal_bldc_3pwm.cpp" "port/angle_sensor/mt6701.cpp" "port/angle_sensor/as5048a.cpp") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@demianzenkov Please run pre-commit install
under this project. Then during git commit
stage, it should auto-find and modify the format or spelling issues.
As you have already added commits, please run SKIP=no-commit-to-branch pre-commit run --files components/motor/esp_simplefoc
to manually run pre-commit on the files you have committed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ran pre-commit
but seems it didn't change any files.
esp-iot-solution % SKIP=no-commit-to-branch pre-commit run --files components/motor/esp_simplefoc
Check copyright notices..............................(no files to check)Skipped
file contents sorter.................................(no files to check)Skipped
trim trailing whitespace.............................(no files to check)Skipped
fix end of files.....................................(no files to check)Skipped
check that executables have shebangs.................(no files to check)Skipped
check that scripts with shebangs are executable......(no files to check)Skipped
mixed line ending....................................(no files to check)Skipped
fix double quoted strings............................(no files to check)Skipped
Do not use more than one slash in the branch name.......................Skipped
Do not use uppercase letters in the branch name.........................Skipped
astyle formatter.....................................(no files to check)Skipped
Check File Permissions...............................(no files to check)Skipped
Validate executable-list.txt.............................................Passed
codespell............................................(no files to check)Skipped
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@demianzenkov Sorry for the Incorrect command, it seems the pre-commit
skipped all checks.
Please run SKIP=no-commit-to-branch pre-commit run --files components/motor/esp_simplefoc/**/*
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it works as normal, #382 (comment) will be fixed automatically
Hello, please merge the current 5 commits into one commit so that we can do the final merge. @demianzenkov |
@YanKE01 commits squashed. |
sha=a491e4b18a206e63e9c69ebd366dd35c37c69934 |
sha=6064126c792384023f5a7a84612d662e577b8143 |
- 14-bit rotary position encoder AS5048A support added
@leeebo license announcement added, commits squashed again. |