Skip to content

Claude/code review thorough 011 c uwa s19d up vate9q kzirx #16

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.

Sign up for GitHub

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

Open
ftrain wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Claude/code review thorough 011 c uwa s19d up vate9q kzirx #16

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dda2c1c
Add comprehensive code review and HardwareBase refactoring
claude Nov 9, 2025
d6c6a24
Refactor: Eliminate duplication and centralize configuration
claude Nov 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
748 changes: 748 additions & 0 deletions docs/CODE_REVIEW.md
View file
Open in desktop

Large diffs are not rendered by default.

583 changes: 583 additions & 0 deletions docs/IMMEDIATE_ACTIONS.md
View file
Open in desktop

Large diffs are not rendered by default.

401 changes: 401 additions & 0 deletions docs/REFACTORING_SUMMARY.md
View file
Open in desktop

Large diffs are not rendered by default.

52 changes: 25 additions & 27 deletions native/GRUVBOK/iOS/IOSHardware.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
#pragma once

#include "../../src/hardware/hardware_interface.h"
#include <array>
#include "../../../../src/hardware/hardware_base.h"
#include <memory>
#include <deque>
#include <string>
Expand All @@ -21,32 +20,35 @@ typedef struct objc_object UIImpactFeedbackGenerator;
namespace gruvbok {

/**
* Native iOS implementation using CoreMIDI and AVAudioEngine
* Optimized for touch input with haptic feedback
* @brief Native iOS implementation using CoreMIDI and AVAudioEngine
*
* Inherits common functionality from HardwareBase and adds:
* - CoreMIDI output (for external hardware)
* - AVAudioEngine for internal synthesis (prioritized on iOS)
* - Haptic feedback for touch interactions
* - Logging functionality
*
* Optimized for touch input with haptic feedback.
*
* @note Inherits button/pot/LED state management from HardwareBase
* @see HardwareBase for inherited functionality
*/
class IOSHardware : public HardwareInterface {
class IOSHardware : public HardwareBase {
public:
IOSHardware();
~IOSHardware() override;

// HardwareInterface implementation
bool init() override;
void shutdown() override;

bool readButton(int button) override;
uint8_t readRotaryPot(int pot) override;
uint8_t readSliderPot(int pot) override;

void sendMidiMessage(const MidiMessage& msg) override;
void setLED(bool on) override;
bool getLED() const override { return led_state_; }
uint32_t getMillis() override;

void update() override;
uint32_t getMillis() override; // Override with mach_absolute_time

// Simulation methods (called from SwiftUI)
void simulateButton(int button, bool pressed);
void simulateRotaryPot(int pot, uint8_t value);
void simulateSliderPot(int pot, uint8_t value);
// Note: Button/pot/LED/simulation methods inherited from HardwareBase
// - readButton(), readRotaryPot(), readSliderPot()
// - setLED(), getLED()
// - simulateButton(), simulateRotaryPot(), simulateSliderPot()

// Audio control (iOS prioritizes internal audio)
bool initAudio(const std::string& soundfont_path = "");
Expand All @@ -66,28 +68,24 @@ class IOSHardware : public HardwareInterface {
bool selectMidiOutput(int index);

private:
// MIDI (optional on iOS - most users use internal audio)
// iOS-specific MIDI state (optional - most users use internal audio)
MIDIClientRef midi_client_;
MIDIPortRef midi_output_port_;
int current_midi_output_;
bool midi_initialized_;
bool use_external_midi_;

// Audio (Objective-C++ objects)
// iOS-specific audio state (Objective-C++ objects)
AVAudioEngine* audio_engine_;
AVAudioUnitSampler* sampler_;
bool audio_initialized_;
bool use_internal_audio_;

// Haptic feedback
// iOS-specific haptic feedback
UIImpactFeedbackGenerator* haptic_generator_;

// Hardware state
std::array<bool, 16> buttons_;
std::array<uint8_t, 4> rotary_pots_;
std::array<uint8_t, 4> slider_pots_;
uint64_t start_time_;
bool led_state_;
// Timing (iOS-specific, overrides base implementation)
uint64_t ios_start_time_; // mach_absolute_time at init

// Logging
std::deque<std::string> log_messages_;
Expand Down
59 changes: 6 additions & 53 deletions native/GRUVBOK/iOS/IOSHardware.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,16 @@
, audio_initialized_(false)
, use_internal_audio_(true) // Default ON for iOS
, haptic_generator_(nullptr)
, led_state_(false) {

buttons_.fill(false);
rotary_pots_.fill(64);
slider_pots_.fill(64);
, ios_start_time_(0) {
// Base class constructor initializes buttons_, pots_, led_state_, start_time_
}

IOSHardware::~IOSHardware() {
shutdown();
}

bool IOSHardware::init() {
start_time_ = mach_absolute_time();
ios_start_time_ = mach_absolute_time();

// Initialize haptic feedback
@autoreleasepool {
Expand Down Expand Up @@ -163,20 +160,8 @@
}
}

bool IOSHardware::readButton(int button) {
if (button < 0 || button >= 16) return false;
return buttons_[button];
}

uint8_t IOSHardware::readRotaryPot(int pot) {
if (pot < 0 || pot >= 4) return 0;
return rotary_pots_[pot];
}

uint8_t IOSHardware::readSliderPot(int pot) {
if (pot < 0 || pot >= 4) return 0;
return slider_pots_[pot];
}
// Note: readButton(), readRotaryPot(), readSliderPot() inherited from HardwareBase
// But we override setLED() and simulateButton() to add haptic feedback

void IOSHardware::sendMidiMessage(const MidiMessage& msg) {
if (use_external_midi_ && midi_initialized_) {
Expand Down Expand Up @@ -230,15 +215,6 @@
}
}

void IOSHardware::setLED(bool on) {
led_state_ = on;

// Trigger haptic feedback on beat (when LED turns on)
if (on) {
triggerHaptic();
}
}

void IOSHardware::triggerHaptic() {
@autoreleasepool {
if (haptic_generator_) {
Expand All @@ -254,7 +230,7 @@
}

uint64_t now = mach_absolute_time();
uint64_t elapsed = now - start_time_;
uint64_t elapsed = now - ios_start_time_;
uint64_t nanos = elapsed * timebase.numer / timebase.denom;
return static_cast<uint32_t>(nanos / 1000000);
}
Expand All @@ -263,29 +239,6 @@
// Nothing to update for iOS (no polling needed)
}

void IOSHardware::simulateButton(int button, bool pressed) {
if (button >= 0 && button < 16) {
buttons_[button] = pressed;

// Haptic feedback on button press
if (pressed) {
triggerHaptic();
}
}
}

void IOSHardware::simulateRotaryPot(int pot, uint8_t value) {
if (pot >= 0 && pot < 4) {
rotary_pots_[pot] = std::min(value, static_cast<uint8_t>(127));
}
}

void IOSHardware::simulateSliderPot(int pot, uint8_t value) {
if (pot >= 0 && pot < 4) {
slider_pots_[pot] = std::min(value, static_cast<uint8_t>(127));
}
}

void IOSHardware::setAudioGain(float gain) {
if (audio_engine_) {
[audio_engine_ mainMixerNode].outputVolume = gain;
Expand Down
46 changes: 21 additions & 25 deletions native/GRUVBOK/macOS/MacOSHardware.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
#pragma once

#include "../../src/hardware/hardware_interface.h"
#include <array>
#include "../../../../src/hardware/hardware_base.h"
#include <memory>
#include <deque>
#include <string>
Expand All @@ -19,31 +18,32 @@ typedef struct objc_object AVAudioUnitSampler;
namespace gruvbok {

/**
* Native macOS implementation using CoreMIDI and AVAudioEngine
* @brief Native macOS implementation using CoreMIDI and AVAudioEngine
*
* Inherits common functionality from HardwareBase and adds:
* - CoreMIDI output (virtual source + physical destinations)
* - AVAudioEngine for internal synthesis
* - Logging functionality
*
* @note Inherits button/pot/LED state management from HardwareBase
* @see HardwareBase for inherited functionality
*/
class MacOSHardware : public HardwareInterface {
class MacOSHardware : public HardwareBase {
public:
MacOSHardware();
~MacOSHardware() override;

// HardwareInterface implementation
bool init() override;
void shutdown() override;

bool readButton(int button) override;
uint8_t readRotaryPot(int pot) override;
uint8_t readSliderPot(int pot) override;

void sendMidiMessage(const MidiMessage& msg) override;
void setLED(bool on) override;
bool getLED() const override { return led_state_; }
uint32_t getMillis() override;

void update() override;
uint32_t getMillis() override; // Override with mach_absolute_time

// Simulation methods (called from SwiftUI)
void simulateButton(int button, bool pressed);
void simulateRotaryPot(int pot, uint8_t value);
void simulateSliderPot(int pot, uint8_t value);
// Note: Button/pot/LED/simulation methods inherited from HardwareBase
// - readButton(), readRotaryPot(), readSliderPot()
// - setLED(), getLED()
// - simulateButton(), simulateRotaryPot(), simulateSliderPot()

// Audio control
bool initAudio(const std::string& soundfont_path = "");
Expand All @@ -63,26 +63,22 @@ class MacOSHardware : public HardwareInterface {
bool selectMidiOutput(int index);

private:
// MIDI
// macOS-specific MIDI state
MIDIClientRef midi_client_;
MIDIPortRef midi_output_port_;
MIDIEndpointRef midi_virtual_source_;
int current_midi_output_;
bool midi_initialized_;
bool use_external_midi_;

// Audio (Objective-C++ objects)
// macOS-specific audio state (Objective-C++ objects)
AVAudioEngine* audio_engine_;
AVAudioUnitSampler* sampler_;
bool audio_initialized_;
bool use_internal_audio_;

// Hardware state
std::array<bool, 16> buttons_;
std::array<uint8_t, 4> rotary_pots_;
std::array<uint8_t, 4> slider_pots_;
uint64_t start_time_;
bool led_state_;
// Timing (macOS-specific, overrides base implementation)
uint64_t macos_start_time_; // mach_absolute_time at init

// Logging
std::deque<std::string> log_messages_;
Expand Down
50 changes: 7 additions & 43 deletions native/GRUVBOK/macOS/MacOSHardware.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,16 @@
, sampler_(nullptr)
, audio_initialized_(false)
, use_internal_audio_(false)
, led_state_(false) {

buttons_.fill(false);
rotary_pots_.fill(64); // Default middle value
slider_pots_.fill(64);
, macos_start_time_(0) {
// Base class constructor initializes buttons_, pots_, led_state_, start_time_
}

MacOSHardware::~MacOSHardware() {
shutdown();
}

bool MacOSHardware::init() {
start_time_ = mach_absolute_time();
macos_start_time_ = mach_absolute_time();

// Initialize CoreMIDI
OSStatus status = MIDIClientCreate(CFSTR("GRUVBOK"), nullptr, nullptr, &midi_client_);
Expand Down Expand Up @@ -150,20 +147,9 @@
}
}

bool MacOSHardware::readButton(int button) {
if (button < 0 || button >= 16) return false;
return buttons_[button];
}

uint8_t MacOSHardware::readRotaryPot(int pot) {
if (pot < 0 || pot >= 4) return 0;
return rotary_pots_[pot];
}

uint8_t MacOSHardware::readSliderPot(int pot) {
if (pot < 0 || pot >= 4) return 0;
return slider_pots_[pot];
}
// Note: readButton(), readRotaryPot(), readSliderPot(), setLED(), getLED(),
// simulateButton(), simulateRotaryPot(), simulateSliderPot()
// are now inherited from HardwareBase

void MacOSHardware::sendMidiMessage(const MidiMessage& msg) {
if (use_external_midi_ && midi_initialized_) {
Expand Down Expand Up @@ -233,18 +219,14 @@
}
}

void MacOSHardware::setLED(bool on) {
led_state_ = on;
}

uint32_t MacOSHardware::getMillis() {
static mach_timebase_info_data_t timebase;
if (timebase.denom == 0) {
mach_timebase_info(&timebase);
}

uint64_t now = mach_absolute_time();
uint64_t elapsed = now - start_time_;
uint64_t elapsed = now - macos_start_time_;
uint64_t nanos = elapsed * timebase.numer / timebase.denom;
return static_cast<uint32_t>(nanos / 1000000);
}
Expand All @@ -253,24 +235,6 @@
// Nothing to update for macOS (no polling needed)
}

void MacOSHardware::simulateButton(int button, bool pressed) {
if (button >= 0 && button < 16) {
buttons_[button] = pressed;
}
}

void MacOSHardware::simulateRotaryPot(int pot, uint8_t value) {
if (pot >= 0 && pot < 4) {
rotary_pots_[pot] = std::min(value, static_cast<uint8_t>(127));
}
}

void MacOSHardware::simulateSliderPot(int pot, uint8_t value) {
if (pot >= 0 && pot < 4) {
slider_pots_[pot] = std::min(value, static_cast<uint8_t>(127));
}
}

void MacOSHardware::setAudioGain(float gain) {
if (audio_engine_) {
[audio_engine_ mainMixerNode].outputVolume = gain;
Expand Down
Loading