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

Refactoring #7

Open
puzrin opened this issue Oct 23, 2021 · 5 comments
Open

Refactoring #7

puzrin opened this issue Oct 23, 2021 · 5 comments

Comments

@puzrin
Copy link

puzrin commented Oct 23, 2021

In continue of #6 (comment) and below

See proposal draft for v2.0.0: https://github.com/speedcontrols/dc_sc_grinder/tree/dev/lib/SYLT-FFT

Goal: simplify use of SYLT-FFT as library.

Renamed files/folders

  1. More descriptive.
  2. Auto-compatibility with platformio, arduino etc (those build systems recognize such folders well). No need to add env-specific library declarations.
  3. More extendable (can add test/ folder later)

May be include/sylt-fft/ instead of include? See etl for example.

Added lib namespace prefixes (incomplete)

Example: https://github.com/speedcontrols/dc_sc_grinder/blob/dev/lib/SYLT-FFT/include/config.h#L24

  1. I did minimal changes, for example only. More prefixing required. Did not touched math, according to your notes.
  2. Not sure about SYLT_FFT_ (too long). May be SYLT_? What "sylt" means?

Redeclared sine table

https://github.com/speedcontrols/dc_sc_grinder/blob/dev/lib/SYLT-FFT/include/config.h#L46-L82

  1. More flexible. Now can add more tables of different length
  2. Allow external const table

Added #ifdef guards (incomplete)

https://github.com/speedcontrols/dc_sc_grinder/blob/dev/lib/SYLT-FFT/include/config.h#L48

That allows to configure everything via -D, no need to edit config manually (+ need prefixes to avoid collisions).

Suppressed arch warnings

https://github.com/speedcontrols/dc_sc_grinder/blob/dev/lib/SYLT-FFT/include/intrinsics.h#L12

Hided warnings behind debug guard. If user use this lib with < M4 - no need to annoy on every build.


Notes:

  • Please, add tag 1.0.0 to last commit. That will help users to access "previous" version, and creating CHANGELOG.md.
  • First refactoring iteration require 2 things:
    • reorganize folders/files
    • decide final name for "library namespace prefixes". SYLT_FFT_? SYLT_? Other?

What do you think? Can you propagate acceptable changes to upstream?

PS. I'm ~ software architect, but NOT C programmer. So, i'd prefer to not intrude to your code via PRs, without experience of C specifics. But i can prepare samples/descriptions of proposals and other things (except coding).

@puzrin
Copy link
Author

puzrin commented Oct 23, 2021

One more note about namespaceing. I remember, you wish to have simple math functions accessible.

My proposal is (two parts):

  1. Namespace everything
  2. Create fft_legacy.h file with something like this:
    #define old_name1 namespaced_name1
    #define old_name2 namespaced_name2
    ...

Then:

  • library will be collision-free by default
  • existing users can continue use "old-style" api, enable it with single line #include "fft_legacy.h"

=> all should be happy

@stg
Copy link
Owner

stg commented Oct 23, 2021

Sure thing!

@puzrin
Copy link
Author

puzrin commented Oct 24, 2021

I think we have agreement about principal questions. Can i help somehow more to land changes into master?

@stg
Copy link
Owner

stg commented Mar 25, 2022

Dear puzrin,

I am not able to keep track of activity on my git account.
Please fork the project and take it over, such that you have full control of it.
I will amend the README to reflect this.

@puzrin
Copy link
Author

puzrin commented Mar 25, 2022

@stg consider create organization, transfer project there and add me as second admin of org. Then i'll be able to curate development until your situation changes back.

If i do fork, that will look like parasitism on your work strange :). I'd like to avoid such glory. I'm really impressed by your efforts in sylt-fft and would like project to be continued in best possible way. IMO, org with 2 admins would be optimal.

Also, if you like to keep existing code as single-file-based, we could create second repo sylt-fft2. But again - in context of organization.

Let me know what do you think, or if i misunderstand something.

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

No branches or pull requests

2 participants