Proposal: Replace numerical values in examples with preprocessor macros from ch32v003fun.h

[prosper00]

I notice that most of the examples ignore #defined values from the header file, in favour of simply using a number; example:

GPIOC->CFGLR &= ~(0xf<<(4*1)); instead of GPIOC->CFGLR &= ~(0xf<<(4*GPIO_PinSource1)); where this already exists in the header: ch32v003fun/ch32v003fun.h:3428:#define GPIO_PinSource1 ((uint8_t)0x01)

Or: ADC1->SAMPTR2 |= 7<<(3*7); // 0:7 => 3/9/15/30/43/57/73/241 cycles instead of ADC1->SAMPTR2 |= ( ADC_SampleTime_241Cycles << (3 * ADC_Channel_7 ) ); // 0:7 => 3/9/15/30/43/57/73/241 cycles

My asks: I feel like using the macros makes for more easily understood code, and that (for the purposes of 'canonical' examples), this is a desirable thing. Does anyone else feel like this is a useful thing? and if so:

1. I propose the creation of a 'style guide' for examples and contributions, stressing the use (where possible) of macros instead of digits
2. I'm not much of a coder, but I can start grooming the existing examples when I have time and motivation, and opening up PRs. Would this be acceptable?

[dwillmore]

I think things evolved pretty quickly and not all examples were changed when new notations were added--or the person writing the example wasn't aware of the convention.

So, a style guide is a great idea, but for programming that's usually in the actual code of the examples. So you could write a short set of rules for best practices, but the real useful contribution would be altering the examples to conform to these best practices.

You're free to start with the examples I contributed! I would love to see my code look less horrible.

On Tue, Jun 13, 2023 at 11:10 AM prosper00 @.***> wrote:

I notice that most of the examples ignore #defined values from the header file, in favour of simply using a number; example:

GPIOC->CFGLR &= ~(0xf<<(41)); instead of GPIOC->CFGLR &= ~(0xf<<(4GPIO_Pin_1)); where this already exists in the header: ch32v003fun.h:3393:#define GPIO_Pin_1 ((uint16_t)0x0002) /* Pin 1 selected */

Or: ADC1->SAMPTR2 |= 7<<(3*7); // 0:7 => 3/9/15/30/43/57/73/241 cycles instead of ADC1->SAMPTR2 |= ( ADC_SampleTime_3Cycles << (3 * ADC_Channel_2 ) ); // 0:7 => 3/9/15/30/43/57/73/241 cycles

My asks: I feel like using the macros makes for more easily understood code, and that (for the purposes of 'canonical' examples), this is a desirable thing. Does anyone else feel like this is a useful thing? and if so:

1. I propose the creation of a 'style guide' for examples and contributions, stressing the use (where possible) of macros instead of digits
2. I'm not much of a coder, but I can start grooming the existing examples when I have time and motivation, and opening up PRs. Would this be acceptable?

— Reply to this email directly, view it on GitHub https://github.com/cnlohr/ch32v003fun/issues/171, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACPEX7GTPDVD3MROP5QPZITXLB7EXANCNFSM6AAAAAAZFBCPYY . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[prosper00]

I spend a lot of time with grep-ing for structures and macros that I can use wherever possible. At least they follow a somewhat predictable and consistent naming convention. grep -rin adc ch32v003fun/*

I'll start prettying up a few of the examples when I get some time. Maybe I'll write a script to semi-automate things.

Thanks.

[mengstr]

consts/defines are usually nice - as long as they don't cause confusion when trying to make heads or tails of they code you're reading and at the same time looking at the datasheet. Compare the defines here with the names of the corresponding bits in the datasheet.

I had to hover over them in the code to get the popup with their hex values, and then matching the hex value to the bit-number in the dox so I could see the description.

I mean - how is even CR_OPTPG_Set close to OBER - that is the real name from the datasheet?

#define CR_PAGE_ER                 ((uint32_t)0x00020000)
#define CR_PAGE_PG                 ((uint32_t)0x00010000)
#define CR_OPTER_Set               ((uint32_t)0x00000020)
#define CR_OPTPG_Set               ((uint32_t)0x00000010)

[cnlohr]

We are poised to make some of these decisions. In general I have gone in the direction of more permissive instead of more constrained regarding style. I.e. where constants in place make sense, just use them, but keep the things from the EVT available for anyone copy-pasting code.

I am also frustrated by the tire fire that the flash system definitions are :(. The reference manual is missing an entire register :(