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

Bluetooth: Periodic Advertising, Filter Accept List, Resolving list related variable name abbreviations #39025

Closed
cvinayak opened this issue Oct 1, 2021 · 4 comments
Labels
area: Bluetooth Enhancement Changes/Updates/Additions to existing features

Comments

@cvinayak
Copy link
Contributor

cvinayak commented Oct 1, 2021

Is your enhancement proposal related to a problem? Please describe.
Periodic Advertising, Periodic Advertising List, Filter Accept List, and Resolving List use abbreviations per_adv, pal, fal and rl respectively, which are "It is hard to figure out what is it until you are in the topic."

  • Originally, pl was used for Periodic Advertising List as pal would overlapped with 802.11 PAL in BT Spec
  • pa is popular for Power Amplifier
  • PER is abbreviation for Peripheral in the Bluetooth Test Specification
  • per is a word in itself having different meaning, like in "as per" something

Refer to discussions here:

Describe the solution you'd like

  • "So if not pal and not per_adv_list, I guess we are just left with pa_list or periodic_adv_list (or even periodic_advertiser_list)."

Baseline the variable abbreviations in this issue, and take actions to refactor implementation to reflect the decided namings.

Describe alternatives you've considered
Keep the per_adv, pal, fal and rl until a consensus is reached on an adopted rules for variable naming

Additional context
None for now.

@cvinayak cvinayak added the Enhancement Changes/Updates/Additions to existing features label Oct 1, 2021
@carlescufi
Copy link
Member

I see no real reason not to use pal, fal and rl. Those are the literal abbreviations of the spec terms, and this is internal code, not public APIs. My vote goes to staying with those.

@ppryga-nordic
Copy link
Collaborator

I've got different opinion on that.

Even the code is internal it should be as readable as possible. That would not intimidate new developers that want to contribute to the implementation.
The abbreviations are not used in BT Core Spec. @Thalley can confirm that, because he has verified it.
What more there is a number of abbreviations used in the Bluetooth stack. These are ok for people that are already experienced in development of the stack and read the code related with periodic advertiser list, filter accept list, resolving list. If one hadn't been familiar with the features of the protocol and hadn't done anything in that areas of the stack, then it would be hard to guess what the abbreviations mean.

In my humble opinion, we should avoid making steeper already steep learning curve of the stack implementation.

@Thalley
Copy link
Collaborator

Thalley commented Oct 1, 2021

Furthermore, pal (PAL) was the spec acronym for the Protocol Adaptation Layer from Core 5.2 and earlier (remove in 5.3).

I personally think fal and pal are fine to use, but I also agree with @ppryga that they may be a bit tricky.

@jori-nordic
Copy link
Collaborator

Closing as won't do since it's been more than a year and people are mostly okay with keeping the current abbrevs.

I opened #54893 instead, we'll try to have a first version in the next few months.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Bluetooth Enhancement Changes/Updates/Additions to existing features
Projects
None yet
Development

No branches or pull requests

6 participants