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

added first implementation of utils sub-module #342

Merged
merged 8 commits into from
Mar 8, 2021
Merged

added first implementation of utils sub-module #342

merged 8 commits into from
Mar 8, 2021

Conversation

v923z
Copy link
Owner

@v923z v923z commented Mar 3, 2021

This PR should address the issue raised in #341

The general problem is that peripheral devices do not necessarily supply data compatible with ulab's 5 supported dtypes. However, for the most common tasks, simple conversion functions can be added. With the help of these, arbitrary data formats could be converted to the supporetd dtypes. After the conversion, the standard ulab operations can be carried out on the data arrays.

In the future, the utils sub-module could be the place, where useful, but not numpy-compatible functions working with or returning ndarrays could be stored.

Copy link

@pumelo pumelo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably the cause for the exception I saw.. About to recompile and test .. 🏃

}
}
} else {
for(uint8_t i = 0; i < len; i++) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use size_t? uint8_t will probably not reach the end condition...

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this was the issue! with the documented crash.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed! 😊 Thanks for catching this!

@v923z v923z marked this pull request as ready for review March 5, 2021 07:32
@v923z
Copy link
Owner Author

v923z commented Mar 5, 2021

@ubIQio This can also solve your issue #306

@v923z v923z merged commit c147c90 into master Mar 8, 2021
@v923z v923z deleted the utils branch March 8, 2021 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants