From 56936aed10fd8e6179a07a597cf1f8195e02665d Mon Sep 17 00:00:00 2001 From: Allen Hill Date: Fri, 17 Jul 2026 14:31:54 -0700 Subject: [PATCH] Switch to using references where possible --- src/avclan/bus.cc | 18 +++--- src/avclan/bus.hpp | 2 +- src/avclan/cdchanger.cc | 122 +++++++++++++++++++------------------- src/avclan/cdchanger.hpp | 10 ++-- src/avclan/device.hpp | 2 +- src/avclan/peripheral.hpp | 38 ++++++------ 6 files changed, 96 insertions(+), 96 deletions(-) diff --git a/src/avclan/bus.cc b/src/avclan/bus.cc index df0a562..720f068 100644 --- a/src/avclan/bus.cc +++ b/src/avclan/bus.cc @@ -324,7 +324,7 @@ auto Bus::read(uint16_t address, Frame::Print print) return in; } -auto Bus::send(const Frame *out, Frame::Print print) -> Send { +auto Bus::send(const Frame &out, Frame::Print print) -> Send { struct errtype { // Error enum is ordered such that a lower numeric value corresponds to // more success @@ -348,35 +348,35 @@ auto Bus::send(const Frame *out, Frame::Print print) -> Send { goto handle_err; } - handle.send<1>(static_cast(out->is_unicast), no_parity); + handle.send<1>(static_cast(out.is_unicast), no_parity); - handle.send<12>(out->controller_addr, with_parity); + handle.send<12>(out.controller_addr, with_parity); if (auto serr = - handle.send<12>(out->peripheral_addr, with_ack, out->is_unicast); + handle.send<12>(out.peripheral_addr, with_ack, out.is_unicast); serr == NAK) { err.errno = NAK_ADDRESS; goto handle_err; } - if (auto serr = handle.send<4>(out->control, with_ack, out->is_unicast); + if (auto serr = handle.send<4>(out.control, with_ack, out.is_unicast); serr == NAK) { err.errno = NAK_CONTROL; goto handle_err; } - if (auto serr = handle.send<8>(out->length, with_ack, out->is_unicast); + if (auto serr = handle.send<8>(out.length, with_ack, out.is_unicast); serr == NAK) { err.errno = NAK_MESSAGE_LENGTH; goto handle_err; } - for (uint8_t i = 0; i < out->length; i++) { + for (uint8_t i = 0; i < out.length; i++) { // Based on the µPD6708 datasheet, ACK bit for broadcast doesn't seem // necessary (i.e. This deviates from the previous broadcast specific // function that sent an extra `1` bit after each byte/parity) // Explanation for why audio-group broadcast state report isn't working? - if (auto serr = handle.send<8>(out->data[i], with_ack, out->is_unicast); + if (auto serr = handle.send<8>(out.data[i], with_ack, out.is_unicast); serr == NAK) { err.errno = NAK_DATA; err.val = i; @@ -413,7 +413,7 @@ auto Bus::send(const Frame *out, Frame::Print print) -> Send { } if (print.print) - out->print(print); + out.print(print); return err.errno; } diff --git a/src/avclan/bus.hpp b/src/avclan/bus.hpp index c5b0046..bb2a6d9 100644 --- a/src/avclan/bus.hpp +++ b/src/avclan/bus.hpp @@ -81,7 +81,7 @@ public: expected, Error::Read> read(uint16_t address, Frame::Print print); - Error::Send send(const Frame *out, Frame::Print print); + Error::Send send(const Frame &out, Frame::Print print); private: class Handle; diff --git a/src/avclan/cdchanger.cc b/src/avclan/cdchanger.cc index 11903d4..2c296b0 100644 --- a/src/avclan/cdchanger.cc +++ b/src/avclan/cdchanger.cc @@ -57,11 +57,11 @@ void CDChanger::init() { cdtimer_init(this, &incrementTime_callback, &isPlaying_callback); } -void CDChanger::handle(const Frame *in, Frame *out) { - if (in->length < 4) +void CDChanger::handle(const Frame &in, Frame &out) { + if (in.length < 4) return; // [Currently known] valid CDChanger frames have at least 4 bytes - const uint8_t *data = &in->data[1]; + const uint8_t *data = &in.data[1]; const auto from = static_cast(*data++); /* const auto to = */ data++; const auto action = static_cast(*data++); @@ -75,25 +75,25 @@ void CDChanger::handle(const Frame *in, Frame *out) { // Unicast to CD changer: bytes are (0x00, from, to, action, [extra...]). switch (action) { case Enable_Function_Req: - out->is_unicast = true; - out->length = sizeof(function_change_resp); - memcpy(out->data, function_change_resp, sizeof(function_change_resp)); - out->data[3] = to_underlying(Enable_Function_Resp); + out.is_unicast = true; + out.length = sizeof(function_change_resp); + memcpy(out.data, function_change_resp, sizeof(function_change_resp)); + out.data[3] = to_underlying(Enable_Function_Resp); state = 0; flags2 = 0x80; - out->reaction = r_StatusReport; + out.reaction = r_StatusReport; break; case Disable_Function_Req: // No change/response needed if we're already not playing if (isPlaying()) { stopPlaying(); - out->length = sizeof(function_change_resp); - memcpy(out->data, function_change_resp, sizeof(function_change_resp)); - out->data[3] = to_underlying(Disable_Function_Resp); + out.length = sizeof(function_change_resp); + memcpy(out.data, function_change_resp, sizeof(function_change_resp)); + out.data[3] = to_underlying(Disable_Function_Resp); state = 0; flags2 = 0x80; - out->is_unicast = true; - out->reaction = r_StatusReport; + out.is_unicast = true; + out.reaction = r_StatusReport; } break; case Eject: { @@ -107,20 +107,20 @@ void CDChanger::handle(const Frame *in, Frame *out) { if (static_cast(state & SEEKING)) { // FF/RW button released state &= ~SEEKING; } else { - out->is_unicast = true; + out.is_unicast = true; { const uint8_t msg[] = {0x00, to_underlying(Device::CD_CHANGER), to_underlying(Device::CMD_SW), to_underlying(Insertion), 0x01}; - out->length = sizeof(msg); - memcpy(out->data, msg, sizeof(msg)); + out.length = sizeof(msg); + memcpy(out.data, msg, sizeof(msg)); } - out->reaction = r_SendOnly; + out.reaction = r_SendOnly; } break; } case Initial_Report_Req: { - out->is_unicast = true; + out.is_unicast = true; // No knowledge/understanding of field meaning/interpretation const uint8_t cdinitreport_resp[] = {0x00, to_underlying(Device::CD_CHANGER), @@ -131,29 +131,29 @@ void CDChanger::handle(const Frame *in, Frame *out) { 0x10, 0x01, 0x01}; - out->length = sizeof(cdinitreport_resp); - memcpy(out->data, cdinitreport_resp, sizeof(cdinitreport_resp)); - out->reaction = r_SendOnly; + out.length = sizeof(cdinitreport_resp); + memcpy(out.data, cdinitreport_resp, sizeof(cdinitreport_resp)); + out.reaction = r_SendOnly; break; } case Playback_Req: - out->data[0] = 0x00; - out->data[1] = to_underlying(Device::CD_CHANGER); - out->data[2] = to_underlying(from); - out->data[3] = to_underlying(Playback_Resp); - out->length = WIRE_SIZE + 4; - serialize(&out->data[4]); - out->is_unicast = true; - out->reaction = r_SendOnly; + out.data[0] = 0x00; + out.data[1] = to_underlying(Device::CD_CHANGER); + out.data[2] = to_underlying(from); + out.data[3] = to_underlying(Playback_Resp); + out.length = WIRE_SIZE + 4; + serialize(&out.data[4]); + out.is_unicast = true; + out.reaction = r_SendOnly; break; case Loading_Req: - out->data[0] = 0x00; - out->length = sizeof(cdloading_resp) + 1; - memcpy(&out->data[1], cdloading_resp, sizeof(cdloading_resp)); - out->data[2] = to_underlying(from); - out->data[3] = to_underlying(Loading_Resp); - out->is_unicast = true; - out->reaction = r_SendOnly; + out.data[0] = 0x00; + out.length = sizeof(cdloading_resp) + 1; + memcpy(&out.data[1], cdloading_resp, sizeof(cdloading_resp)); + out.data[2] = to_underlying(from); + out.data[3] = to_underlying(Loading_Resp); + out.is_unicast = true; + out.reaction = r_SendOnly; break; case Track_Seek_Up: state = SEEKING_TRACK; @@ -166,7 +166,7 @@ void CDChanger::handle(const Frame *in, Frame *out) { flags2 = 0xc0; generateStatus(out, true, Device::CMD_SW); media_action(MediaAction::Track_Next); - out->reaction = r_TrackChange; + out.reaction = r_TrackChange; break; case Track_Seek_Down: state = SEEKING_TRACK; @@ -183,7 +183,7 @@ void CDChanger::handle(const Frame *in, Frame *out) { secs = 0x7f; flags2 = 0xc0; generateStatus(out, true, Device::CMD_SW); - out->reaction = r_TrackChange; + out.reaction = r_TrackChange; break; case Track_Fast_Forward: { state |= SEEKING; @@ -196,7 +196,7 @@ void CDChanger::handle(const Frame *in, Frame *out) { media_action(MediaAction::Skip_Forward); cdtimer_reset(); // Skipped to a whole/round sec; ensure next tick // is ~1 sec from now - out->reaction = r_SendOnly; + out.reaction = r_SendOnly; break; } case Track_Rewind: { @@ -216,48 +216,48 @@ void CDChanger::handle(const Frame *in, Frame *out) { media_action(MediaAction::Skip_Backward); cdtimer_reset(); // Skipped to a whole/round sec; ensure next tick // is ~1 sec from now - out->reaction = r_SendOnly; + out.reaction = r_SendOnly; break; } case CD_Enable_Random: flags |= RANDOM; generateStatus(out, true, Device::CMD_SW); - out->reaction = r_StatusReport; + out.reaction = r_StatusReport; break; case CD_Disable_Random: flags &= ~RANDOM; generateStatus(out, true, Device::CMD_SW); - out->reaction = r_StatusReport; + out.reaction = r_StatusReport; break; case CD_Enable_Repeat: flags |= REPEAT; generateStatus(out, true, Device::CMD_SW); - out->reaction = r_StatusReport; + out.reaction = r_StatusReport; break; case CD_Disable_Repeat: flags &= ~REPEAT; generateStatus(out, true, Device::CMD_SW); - out->reaction = r_StatusReport; + out.reaction = r_StatusReport; break; case CD_Enable_Disk_Random: flags |= DISK_RANDOM; generateStatus(out, true, Device::CMD_SW); - out->reaction = r_StatusReport; + out.reaction = r_StatusReport; break; case CD_Disable_Disk_Random: flags &= ~DISK_RANDOM; generateStatus(out, true, Device::CMD_SW); - out->reaction = r_StatusReport; + out.reaction = r_StatusReport; break; case CD_Enable_Disk_Repeat: flags |= DISK_REPEAT; generateStatus(out, true, Device::CMD_SW); - out->reaction = r_StatusReport; + out.reaction = r_StatusReport; break; case CD_Disable_Disk_Repeat: flags &= ~DISK_REPEAT; generateStatus(out, true, Device::CMD_SW); - out->reaction = r_StatusReport; + out.reaction = r_StatusReport; break; default: break; } @@ -308,12 +308,12 @@ std::unique_ptr CDChanger::react( [[fallthrough]]; case r_NormalizeState: normalizeState(); - generateStatus(out, false, Device::STATUS); + generateStatus(*out, false, Device::STATUS); out->reaction = r_SendOnly; break; case r_StartPlaying: normalizeState(); - generateStatus(out, false, Device::STATUS); + generateStatus(*out, false, Device::STATUS); out->reaction = r_BeganPlaying; break; case r_BeganPlaying: @@ -321,7 +321,7 @@ std::unique_ptr CDChanger::react( out->reaction = r_Nothing; break; case r_StatusReport: - generateStatus(out, false, Device::STATUS); + generateStatus(*out, false, Device::STATUS); out->reaction = r_SendOnly; break; case r_StateReport: [[fallthrough]]; @@ -337,7 +337,7 @@ std::unique_ptr CDChanger::react( return {}; } -void CDChanger::enable(Frame *out) { +void CDChanger::enable(Frame &out) { if (!isPlaying()) { if (mins > TWODIGIT_MAX) mins = 0; @@ -346,15 +346,15 @@ void CDChanger::enable(Frame *out) { state = SEEKING | SEEKING_TRACK; flags2 = 0xc0; generateStatus(out, false, Device::STATUS); - out->reaction = r_StartPlaying; + out.reaction = r_StartPlaying; } } bool CDChanger::pending() { return cdtimer_pending(); } -void CDChanger::emit(Frame *out) { +void CDChanger::emit(Frame &out) { generateStatus(out, false, Device::STATUS); - out->reaction = r_StateReport; + out.reaction = r_StateReport; cdtimer_clear(); } @@ -411,15 +411,15 @@ void CDChanger::incrementTime() { } // Used for changed status messages -void CDChanger::generateStatus(Frame *status, bool is_unicast, +void CDChanger::generateStatus(Frame &status, bool is_unicast, Device to) const { - status->is_unicast = is_unicast; + status.is_unicast = is_unicast; if (!is_unicast) - status->peripheral_addr = 0x1FF; - status->control = 0xF; - status->length = WIRE_SIZE + ((is_unicast) ? 4 : 3); + status.peripheral_addr = 0x1FF; + status.control = 0xF; + status.length = WIRE_SIZE + ((is_unicast) ? 4 : 3); - uint8_t *data = status->data; + uint8_t *data = status.data; if (is_unicast) *data++ = 0x00; *data++ = to_underlying(Device::CD_CHANGER); diff --git a/src/avclan/cdchanger.hpp b/src/avclan/cdchanger.hpp index 8bdf845..4f8733e 100644 --- a/src/avclan/cdchanger.hpp +++ b/src/avclan/cdchanger.hpp @@ -61,13 +61,13 @@ public: static constexpr Device id = Device::CD_CHANGER; void init(); - void handle(const Frame *in, Frame *out); + void handle(const Frame &in, Frame &out); std::unique_ptr react(expected, detail::SendError> exp); - void enable(Frame *out); - void disable(Frame *out); + void enable(Frame &out); + void disable(Frame &out); static bool pending(); - void emit(Frame *out); + void emit(Frame &out); void incrementTime(); bool isPlaying() const; #ifndef NDEBUG @@ -81,7 +81,7 @@ private: void stopPlaying(); void serialize(uint8_t *dst) const; void setTime(uint8_t mins, uint8_t secs); - void generateStatus(Frame *status, bool is_unicast, Device to) const; + void generateStatus(Frame &status, bool is_unicast, Device to) const; void normalizeState(); bool playing = false; diff --git a/src/avclan/device.hpp b/src/avclan/device.hpp index d00577f..deaa337 100644 --- a/src/avclan/device.hpp +++ b/src/avclan/device.hpp @@ -50,7 +50,7 @@ enum class Device : uint8_t { template concept DeviceInterface = requires { std::integral_constant{}; } && - requires(T dev, const Frame *in, Frame *out, + requires(T dev, const Frame &in, Frame &out, expected, detail::SendError> exp) { dev.init(); dev.handle(in, out); diff --git a/src/avclan/peripheral.hpp b/src/avclan/peripheral.hpp index 96c1e3b..42acce3 100644 --- a/src/avclan/peripheral.hpp +++ b/src/avclan/peripheral.hpp @@ -63,9 +63,9 @@ public: expected, detail::SendError> send(std::unique_ptr out, Frame::Print print = Frame::Print{}) { // To "forge" a controller_addr, instantiate a new/different Peripheral - stamp(out.get()); + stamp(*out); out->control = 0xF; - auto err = bus.send(out.get(), print); + auto err = bus.send(*out, print); if (err != Error::Send{0}) return unexpected{ detail::SendError{out->owning_device, out->reaction, err}}; @@ -76,11 +76,11 @@ public: #define PACK3(a, b, c) (((uint32_t)(a) << 16) | ((uint32_t)(b) << 8) | (c)) // expected needed to distinguish don't vs can't respond - expected, Error::Read> route(const Frame *in) { + expected, Error::Read> route(const Frame &in) { using enum Device; using enum Action; - if (is_muted() || in->length < 3) + if (is_muted() || in.length < 3) return {}; std::unique_ptr out(new (std::nothrow) Frame); @@ -94,17 +94,17 @@ public: static const uint8_t lancheck_resp[] = {0x00, to_underlying(COMM_CTRL), to_underlying(LAN), 0xFF, 0xFF}; - stamp(out.get()); + stamp(*out); - const uint8_t *data = in->data; + const uint8_t *data = in.data; const uint8_t b0 = *data++; const uint8_t b1 = *data++; const uint8_t b2 = *data++; uint8_t b3 = 0; - if (in->length > 3) // the shortest known/valid messages are 3 bytes long + if (in.length > 3) // the shortest known/valid messages are 3 bytes long b3 = *data++; - if (!in->is_unicast) { + if (!in.is_unicast) { const auto from = b0; const auto to = b1; const auto action = b2; @@ -140,7 +140,7 @@ public: to_underlying(Advertise_Function)): { auto enable_d = [](auto &d, auto &out) { d.enable(out); }; ((Devs::id == static_cast(b3) - ? originate(std::get(devices_), out.get(), enable_d) + ? originate(std::get(devices_), *out, enable_d) : void()), ...); break; @@ -160,8 +160,8 @@ public: to_underlying(List_Functions_Req)): case PACK3(COMMUNICATION_V2, COMM_CTRL, to_underlying(List_Functions_Req)): { - controller_ = in->controller_addr; - stamp(out.get()); // re-stamp now that controller_ is known + controller_ = in.controller_addr; + stamp(*out); // re-stamp now that controller_ is known out->is_unicast = true; const uint8_t list_functions_resp[] = { 0x00, to_underlying(COMM_CTRL), from, @@ -174,10 +174,10 @@ public: // case Restart_Lan: not handled default: break; } - } else if (in->peripheral_addr == address_ && b0 == 0x00) { + } else if (in.peripheral_addr == address_ && b0 == 0x00) { auto handle_d = [&](auto &d, auto &out) { d.handle(in, out); }; ((Devs::id == static_cast(b2) - ? originate(std::get(devices_), out.get(), handle_d) + ? originate(std::get(devices_), *out, handle_d) : void()), ...); } @@ -215,7 +215,7 @@ public: return {}; } - originate(dev, out.get(), [](auto &d, auto &out) { d.emit(out); }); + originate(dev, *out, [](auto &d, auto &out) { d.emit(out); }); return out; }; @@ -249,15 +249,15 @@ public: } private: - template void stamp(Frame *out) const { + template void stamp(Frame &out) const { if constexpr (P == Sender) - out->controller_addr = address_; + out.controller_addr = address_; else - out->peripheral_addr = controller_; + out.peripheral_addr = controller_; } - void originate(DeviceInterface auto &dev, Frame *out, auto &&fill) { - out->owning_device = std::remove_reference_t::id; + void originate(DeviceInterface auto &dev, Frame &out, auto &&fill) { + out.owning_device = std::remove_reference_t::id; stamp(out); // default set FIRST; fill() may override fill(dev, out); }