Skip to content

Music service cleanup#428

Merged
JF002 merged 4 commits intoInfiniTimeOrg:developfrom
jonvmey:music-service-refactor
Jul 2, 2021
Merged

Music service cleanup#428
JF002 merged 4 commits intoInfiniTimeOrg:developfrom
jonvmey:music-service-refactor

Conversation

@jonvmey
Copy link
Copy Markdown
Contributor

@jonvmey jonvmey commented Jun 13, 2021

@Avamander here are the changes I mentioned in #359 plus one or two other small improvements.

  1. Eliminated the need for reinterpret_cast<ble_uuid_t*>ing by passing pointers to the first member of the UUID structs, which is a ble_uuid_t, instead of the struct itself.
  2. Replaced the macro definitions for the UUID values with constexpr functions that return UUID objects directly and used those to initialize them. Also moved these variables to static values in the source file since that's the only place they're referenced.
  3. Moved MusicCallback into the anonymous namespace since it's also only referenced inside of MusicService.cpp
  4. Moved initialization of member variables with fixed constants to class declaration in the header and added explicit initializers for some uninitialized members.

Another thing I was thinking of doing was adding a simple operator== for the UUID objects, which would just call ble_uuid_cmp. That would help make the if-chain in MusicService::OnCommand a lot cleaner to read.

@Avamander
Copy link
Copy Markdown
Collaborator

I plan on testing this, but not sure if I'll manage it within this week.

@jonvmey jonvmey force-pushed the music-service-refactor branch from 3779133 to 0001347 Compare June 26, 2021 03:18
@JF002
Copy link
Copy Markdown
Collaborator

JF002 commented Jul 2, 2021

Nice 👍 I like the way you replaced the macros by constexpr functions!

@JF002 JF002 added this to the Version 1.3 milestone Jul 2, 2021
@JF002 JF002 merged commit 8031cd1 into InfiniTimeOrg:develop Jul 2, 2021
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.

3 participants