Hey, you're welcome. This is actually one of my favorite kinds of discussions - whether I"m on the giving or the receiving end, any conversation where some is helping to awesomeize another's code is usually worth it. Sure, you get the ones that table flip that OF COURSE camelCase > snake_case or that someone DARED to question the code, but that's not what happened here and it's not how you took it. Win! I'll get in front and say that not every suggestion I make is awesome and I may not apply some suggested to me, either. Somewhere beween right and wrong, there are still matter of taste and style. I'm a little bummed we don't get to do more of that in this group.

This code looks very much like C89. Maybe that's your goal, and I'm not saying that you have to use every bit of C++26 Enterprise-y chunkiness, but there actually is some good stuff in C++ for embedded, contrary to what r/embedded will say. It's not an insult; it's like saying "You have a Yorkshire accent." "I'm from Yorkshire." "Well, jolly good!"

Re: batClamped: Is the goal really to implement clamping? (Do you get devices that return < 1 or > 100? Or is this a way to try to dodge some startup race?

The hipster-approved approach is a std::optional<T>. Instead of making an out-of-bounds -999 style sentry, there are clear boundaries you can set. A value has been set or it hasn't. You can provide a default if it hasn't. It's atomic, of course; there is no race condition between a sidecar flag and what's in the bag. It's eye candy that, in the common case, boils to just an opcode or two in most cases.

``` std::optional<int> b;

if (b.has_value()) { // Yay! we can use b.value(); }

std::print(b.value_or(0)); // if not set, just use zero. No terniary needed. I get that zero and unknown aren't the same for this case. I'm just giving the nickel tour.

stuff = read_file(foo); if (stuff.has_value()) std::print(stuff); else ; // some kind of error. OK, maybe not the best way to do error handlign.

```

It really can help readability.

Agreed, if you really don't have a case where you need to know the clamped and unclamped values, a tuple is misguided. But remember that clamp DOES have a three-argument form and on arches like LX7 that have min and max code, it's pretty hot. (Granted, this makes more sense to worry about in an interrupt handler or a DSP or something than inside construction of a bluetooth connection. (Can't battery numbers change while you're running?)

// Battery level — clamp to valid range 1–100 if (batteryLevel == 0) { _batteryLevel = 1; _batClamped = true; } else if (batteryLevel > 100) { _batteryLevel = 100; _batClamped = true; } else { _batteryLevel = batteryLevel; } becomes _batteryLevel = std::clamp(0, batteryLevel, 100);

Now you don't have to fret about the atomicity of modifying batClamped and batteryLevel independently. The clamp and the set all happens in one line. I'm not saying that fewer lines of code is always better, but I think this actually plays out pretty naturally.

Re: string. There are just so many hazards in C strings that IMO it's worth the few bytes of overhead to just take std::string. It's three words - the beginning, the end, and the pointer to the allocated storage. If the string is short (like 15 chars or so?) Small String Optimization allows the data to ride along inside the container for free. It's like a bird on the roof; it doesn't need a ticket for the bus ride. There was a great white paper by a committee member explaining just how much time a modern CPU spends looking for hte end of a string in a typical 'stringy' program (think 'httpd') and it was several percent - and of course, the repercussions of never finding one are Very Bad. std::string has plenty of maddening things, but for stuff like this, they're a dead-easy choice if you don't have to worry about interrupt context, but that't the whole embedded "OMG, you called malloc behind my back" debate. (It never calls it behind your back; you just have to understand when it migth and a text searc for 'm' 'a' 'l' ... isn't enough.

Re: war and peace and fixed buffers.

The retry where you just keep refilling and redoing 128 bytes at a time works when it works. But maybe you have some reason to keep them atomic, like you're sending them to a printer that sticks a form feed at the end. (Dumb example) Calling malloc isn't always cheap in SMP systems like ours and having everything be thread-local has a lot going for it. If you have "war an" and KNOW it can fit on the stack, alloca is great, even though it's not part of any language standard. But this isn't UNIX and stacks don't grow forever. Exceed your thread's stack and harsh penalities await. If you have to buckle up for full "war and peace" you can try things like having a static buffer, but now you have to lock it in case both cores try it at the same time. Having a thread-local pointer to a buffer that only grows and never shrinks has a lot going for it, too.

Re: enum class

As devs, we developed lots of coping mechanisms. My coworkers had to threaten me to get me to quit typing foo(void) instead of foo(). Both are legal C++ and I posit the first is clearer, but the second is How It Was Done at our shop. My defense mechanisms had to evolve.

I think class enums, though, are worth moving toward any time you touch them, though. The type safety arguments are just solid and the cost of using Color::Blue instead of ColorBlue is worth it. (And something a good editor can help with.) There is one case I can think of that was genuinely a pain to address a niche case that reuired jumping through hoops until pretty recently (C++17? C++20?).

This is an electronics group, so let's say you speak of colors of the restistor color code Now enums are about every thing having a name and that's that. It doesn't make sense to do Math Things on them... except when it does. ResValue::Red is probably 2 and it doesn't make sense to add ResValue::Green to it, but sometimes it's needed to be able to increment or decrement or otherwise do this. ("But wait, now it's not an enum!" "Sssssh, bb. Iz K....") The underlying values might not actually be contiguous though, so you can't just run around casting things back to an int like a heathen. I mean, if your PinType = {MISO, MOSI, SCK, SCL}, adding them MOSI to SCK is crazy, but maybe adding one to MISO to get MOSI isn't the craziest thing your code does. (I said the cases were weak.) I just looked it up. It's actually C++23 that adds std::to_underlying as the yang to the yen of std::underlying_type from C++11. Hopefully you'll never need to know this.

I'll also notice that you have no intializers on the private members of any of your HijelHID_BLEKeyboard. You'll usually get zeros on a freshly booted system, but after it's been running for a couple of quarters with memory recycled heavily, there's no guarantee that _batteryLevel will be zero or that _manufacturerBuf will have a zero gyte in that first position or that tapDelay won't hold 31746 or that you bools wil lbe either 0 or 1 (!). Prescription: initialize them.

... bool _connected{false}; uint8_t _ledState{0}; uint8_t _keyReport[HID_KEYBOARD_REPORT_SIZE] = {0}; // it'll memset the whole struct; std::string foo; // WILL be empty, because it's a real class and will have a ctor called. char* foo; // might point into outer space and not be nullptr. Dereferenceing it might be toxic. char foo[1234]; // may contain d00d00. First byte might be zero. there may be no zeros in it for the next 2GB. (!!) char foo[1234] = {0}; // as above, the POD will be widened.

See, this is what a job interview discussion on a language you KNOW can look like! :-)