Skip to content

Fluidics refactor - #10

Open
Prattbuw wants to merge 73 commits into
mainfrom
fluidics-refactor
Open

Fluidics refactor#10
Prattbuw wants to merge 73 commits into
mainfrom
fluidics-refactor

Conversation

@Prattbuw

Copy link
Copy Markdown
Collaborator

Refactor to support new fluidics system for the delphi behavioral apparatus.

@Prattbuw
Prattbuw requested a review from Poofjunior April 30, 2026 20:03
Comment thread firmware/inc/config.h
@@ -2,18 +2,34 @@
#define CONFIG_H

#define NUM_VALVES (16)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use inline constexpr size_t instead where possible.


// Setup for Harp App
inline constexpr size_t APP_REG_COUNT = 74;
// Numeric addresses for Harp Registers (clunky) -- DO ALL NEW REGISTERS NEED TO BE REFERENCED TO THESE??

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can get rid of these (in most cases) if we use this pattern inside of a read_any handler function:

// Convert address to output channel with pointer arithmetic.
const RegSpec& specs = HarpCore::reg_address_to_spec(address);
size_t channel = ((uint16_t*)specs.base_ptr - app_regs.analog_output_port_state);

It's not pretty, but it works.

The idea is that we already have the base_ptr, which is the array base address, so with pointer arithmetic, we can access everything else, since the data is contiguous. You just have to cast the pointer to the appropriate array element type.

(You probably want to use a static_cast instead of a c-style cast.)

Also: this function (HarpCore::reg_address_to_spec) might be specs not spec in the old core. Just FYI!

Comment thread firmware/inc/delphi_controller_app.h Outdated
#endif

// Setup for Harp App
inline constexpr size_t APP_REG_COUNT = 74;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use either sizeof(array)/sizeof(element) or std::size here in the delphi_controller.cpp.

See this example in the quac board (though I should've used std::size).

/**
* \brief callback function to alert when the leak status changes
*/
void leak_state_alert(void);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend a convention where you name functions that are callback functions as either: something_callback or something_cb or something_handler just to tip off future-you (and others!).


void read_pid_update_frequency(uint8_t reg_address);
void read_pid_gains(uint8_t reg_address);
void read_proportional_valve_0_adc(uint8_t reg_address);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend refactoring this to a read_any_*/write_any_* handler function style, so you don't have to copy the function body thrice.

void write_max_odor_delivery_time_us(msg_t& msg);
void write_minimum_poke_time_us(msg_t& msg);
void write_odor_dwell_time_us(msg_t& msg);
void write_cam0_frame_rate(msg_t& msg);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend a read_any_*/write_any_* pattern for the cam_0 and cam_1 functionality.


void write_pid_update_frequency(msg_t& msg);
void write_pid_gains(msg_t& msg);
void write_proportional_valve_0_adc(msg_t& msg);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment here.

@@ -0,0 +1,232 @@
// Flow detection class spi

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per our in person discussion, here's another way to implement this.

We can setup a class to memory map a continuously updated result that other entities can scoop up on their own time.

Here's an example of streaming 5 (4 + 1 internal) ADC values (4 pins + internal temp) to a fixed location in memory that gets constantly refreshed as fast as the adc can stream it.

The consideration you need to deal with is that you need to read it in one cycle and work off a copy. (The Treadmill does this.) If you can't do this, you need to push the data to a queue or possibly use std::atomic.

Refactoring in this way lets you make one flow detector instance per adc pin rather than having to think of them conceptually as a group.

Image

Comment thread firmware/inc/pwm_pio.h
@@ -0,0 +1,181 @@
// ---------------------------------------------------------------- //

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per chatting and possibly moving this to core1, here are 2 examples.

If your interface is multicore safe, then you don't need to deal with passing settings back and forth between cores (a la the quac board), and your core1 loop becomes very simple. If not ( a la the cuttlefish), then you need to use queues or mutex locks to make sure the shared resources are multicore-safe.

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