Skip to content

Conversation

@Jjm321814
Copy link

This is part of a large array of PR to add blip objects to CAF files.
Previously the key blip structs were defined in sbndcode and this PR simply moves them to sbnobj.

@Jjm321814 Jjm321814 self-assigned this Nov 10, 2025
@Jjm321814 Jjm321814 requested a review from sungbinoh November 10, 2025 15:09
@Jjm321814
Copy link
Author

An associated branch in sbndcode (also feature/AddingBlipToCAF) has the updated libraries linking necessary to point here. Once this one is in a release we can delete the file copies in sbndcode

@kjplows
Copy link
Contributor

kjplows commented Nov 12, 2025

Hi @Jjm321814 , is there a required PR in sbndcode to accompany this? If yes, please point me to that PR. Thanks!

@kjplows
Copy link
Contributor

kjplows commented Nov 12, 2025

Also summoning @PetrilloAtWork to take a look if there's time, not sure if ICARUS has something similar to this?

@Jjm321814
Copy link
Author

The related sbndcode changes are in SBNSoftware/sbndcode#871 but they depend on this PR, not the other way around.
This one just moves some struct definitions out of sbndcode and into sbnobj so that it can be imported into sbncode for CAFMaker.

@Jjm321814
Copy link
Author

Adding a comment to each of these to track all 4 related PR.
sbncode: SBNSoftware/sbncode#603
sbndcode: SBNSoftware/sbndcode#871
sbnobj: #155
sbnanaobj: SBNSoftware/sbnanaobj#173

The changes to sbnobj, and sbnanaobj are fully independent of any other changes, so they can be approved first.

sbndcode changes rely on sbnobj, so it will have to wait for the first approval. A later simple PR will delete the (now duplicated) class files in the BlipUtils folder here

sbncode changes rely on both sbnobj and sbnanaobj, so that will have to wait for both of the first two approvals.

@Jjm321814
Copy link
Author

Accidentally ran with the wrong blip tag and still produced a CAF file in sbndcode area, so I expect it will work with ICARUS but I can not test it.

@Jjm321814
Copy link
Author

All the suggestions here have been committed. Every struct has doxygen comments, TVector3 has been generally replaced, and the few housekeeping comments have been accepted.
A later fix can remove TVector3 from blip processing steps too. May I ask what is the issue with TVector3?

@kjplows kjplows moved this from Open pull requests to Partially reviewed in SBN software development Dec 14, 2025
@kjplows
Copy link
Contributor

kjplows commented Dec 14, 2025

Hi @PetrilloAtWork , it looks like Jacob has implemented all the changes requested. How does the PR look now? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Partially reviewed

Development

Successfully merging this pull request may close these issues.

6 participants