Skip to content

Conversation

@addrian-77
Copy link

@addrian-77 addrian-77 commented Oct 7, 2025

Pull Request Overview

This pull request adds a TBF file filter functionality

TODO or Help Wanted

Checks

Using Rust tooling

  • Ran cargo fmt
  • Ran cargo clippy
  • Ran cargo test
  • Ran cargo build

Features tested:

  • filter_tbfs on Rust and C tab files

GitHub Issue

This pull request closes #114

Signed-off-by: addrian-77 <[email protected]>
@addrian-77 addrian-77 requested a review from eva-cosma October 7, 2025 07:28
@addrian-77 addrian-77 self-assigned this Oct 7, 2025
Comment on lines +107 to +111
fn split_arch(filename: String) -> (String, u64, u64) {
// filename is always formatted like this:
// "cortex-m0.0x10020000.0x20004000.tab"
// splitting by .0x will give us "arch", "flash start", "ram start.tab"
// 3 items
Copy link
Collaborator

Choose a reason for hiding this comment

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

Make these doc comments, and add unit tests for this (you can ask Copilot for this). Just make sure the doc comments show examples of good input, and bad input. Make doc comments highlight the output type

// "cortex-m0.0x10020000.0x20004000.tab"
// splitting by .0x will give us "arch", "flash start", "ram start.tab"
// 3 items
log::info!("filename {filename}");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we still need this log? We might be able to move it to where split_arch is called to make it make more sense in context

Ok(compatible_tbfs)
}

fn split_arch(filename: String) -> (String, u64, u64) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
fn split_arch(filename: String) -> (String, u64, u64) {
/// ....
struct TbfFilename {
/// <put doc comment here>
pub arch: String,
/// ...
pub flash_address: Option<u64>,
/// ...
pub ram_address: Option<u64>,
}
impl TbfFilename {
/// ...
pub fn new(...) -> Self { ... }
/// ... (this gets you the original file name in the archive)
pub fn to_string(&self) -> String {...}
}
fn parse_tbf_filename(filename: String) -> TbfFilename {

The type is quite complex already, let's just make it a struct

Copy link
Collaborator

Choose a reason for hiding this comment

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

and also renamed the function


struct TbfFile {
pub struct TbfFile {
pub filename: String,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
pub filename: String,
pub filename: TbfFilename,

This does mean we do this parsing in Tab::open

Comment on lines +79 to +82
pub fn filter_tbfs(
&self,
settings: &BoardSettings,
) -> Result<Vec<(&TbfFile, u64, u64)>, TockloaderError> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
pub fn filter_tbfs(
&self,
settings: &BoardSettings,
) -> Result<Vec<(&TbfFile, u64, u64)>, TockloaderError> {
/// ...
pub fn compatible_tbfs(
&self,
settings: &BoardSettings,
) -> impl Iterator<item = TbfFile> { // Maybe Item = &TbfFile, unsure, play around

You can then just make this whole function a big old map

self.tbf_files.iter().map(|tbf|
    if !tbf.filename.arch == settings.arch.as_ref().unwrap() {
        return None;
    }
 
    match (tbf.filename.flash_address, tbf.filename.ram_address) {
        (None, None) => Some(tbf),
        (Some(flash), Some(ram)) => { ... some if flash >= and ram >=, otherwise None }
        _ => unreachable!("TbfFilename should not have only ram or only flash address speecified")
    }

}
}

pub fn extract_binary(&self, arch: &str) -> Result<Vec<u8>, TockloaderError> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we yeet this now that we have compatible_tbfs? Let's talk after you finish the other comments

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.

TAB import improvements

3 participants