Naming in ASR650x-Arduino Core is inconsistent and conflicts with Arduino and common C++ naming conventions

[lnlp]

Below feedback is based on ASR650x-Arduino source code and examples.

Use of consistent programming style and naming conventions are (for multiple reasons) important aspects when writing software. Some well known identifier naming conventions are:

• lowerCamelCase
• UpperCamelCase (aka PascalCase)
• lower_case_separated_with_underscores
• ALL_CAPS

For Arduino it is standard to use lowerCamelCase for function and method names. For C++ in general, ALL_CAPS is primarily used for macro names. A common best-practice is to NOT use underscores in camel casing.

To my surprise the ASR650x-Arduino does NOT apply consistent naming conventions. It uses different naming styles that conflicts with both Arduino naming conventions and common C++ naming conventions.

Functions and method names in the ASR650x-Arduino Core use a mixed combination of lowerCamelCase, UpperCamelCase and ALL_CAPS. In examples both lowerCamelCase and UpperCamelCase are used for naming of user defined functions. This inconsistency is bad-practice, confusing and error-prone. Examples:

• lowerCamelCase (good, Arduino-style): setup(), attachInterrupt() and Serial.begin().

• UpperCamelCase (bad, not Arduino-style): BoardInitMcu(), TimerInit(), Radio.Sleep(), LowPower_Handler().

• ALLCAPS (bad, VERY uncommon)_: RGB_ON(), RGB_OFF().

• Mix of lowerCamelCase and UpperCamelCase (bad) in examples: OnSleep() and wakeup() in example LowPower_WakeUpByGPIO.ino.

Similar inconsistencies exist for naming of variables. Examples:

• bool OVER_THE_AIR_ACTIVATION = LORAWAN_NETMODE; /*ALL_CAPS, VERY uncommon*/
• LoRaMacRegion_t REGION = ACTIVE_REGION; //*ALL_CAPS, VERY uncommon*/
• bool KeepNet = LORAWAN_Net_Reserve; /*UpperCamelCase, not Arduino-style*/

It is important to apply (naming) conventions consistently, and apply conventions that are common for a certain environment (e.g. Arduino framework). An exception to the rule is when external libraries are used. For external libraries one is bound to use the naming that is defined in those libraries, which can result in application source code with mixed naming styles (e.g. LoRaMac-node appears to uses UpperCamelCasing for both functions and variables).

Without documentation it is difficult to check which of the current naming inconsistencies should not exist (because caused by sloppiness/lack of consistency) and should be fixed, and others which are unavoidable because caused by external libraries.

[Heltec-Aaron-Lee]

Thank you for your great suggestions, We will take your suggestions and try to correct this series of issues when the 6502 is released.

[lnlp]

These corrections should be made as soon as possible because this will be a breaking change that impacts application code being written for CubeCell devices. The longer you wait the more code that customers will have to correct for the changes. ASR650x is hardware produced by ASR but ASR650x-Arduino is software (Arduino Core) written by Heltec. Fixing naming in ASR650x-Arduino is not dependent on a the ASR6501 or a newer ASR6502. These corrections should be made asap and be used for all ASR650x MCU's.

[Heltec-Aaron-Lee]

@lnlp I had talked with my colleagues, we will begin the changes when we back to work. Because of the virus, we have to delay the time back to work.

[fredolaredo]

Wish you lot of courage ! I can't imagine what is like ! By the way, I am also quite disappointed with the naming conventions, and agree on the fact that the sooner, less changes we will have to do in the code. For now , just digging into this promising platform. Thx

[wasn-eu]

think that this issue is solved with the last update