Skip to content

Minor tweaks to intensity calculation (code clarity, not function) - #34

Open
lauralindzey wants to merge 2 commits into
mainfrom
lel_intensity_clarification
Open

Minor tweaks to intensity calculation (code clarity, not function)#34
lauralindzey wants to merge 2 commits into
mainfrom
lel_intensity_clarification

Conversation

@lauralindzey

Copy link
Copy Markdown
Collaborator

No description provided.

@lauralindzey
lauralindzey requested a review from amarburg January 27, 2023 20:13
@lauralindzey

Copy link
Copy Markdown
Collaborator Author

Tested on Porcine with live data.

I'm not sure why CI is failing -- there's nothing in the logs at all.

@amarburg

Copy link
Copy Markdown
Collaborator

See Zulip

@ALL I need to do some system maintenance this weekend. The in-house CI system will be offline until early next week.

@lauralindzey

Copy link
Copy Markdown
Collaborator Author

Ah, OK! Hadn't realized that applied to CI run from Github.

@amarburg

Copy link
Copy Markdown
Collaborator

OK, I will ask myself a dumb question. Why does sonar_postprocess exist? Right now it is a ProjectSonarImage ->ProjectedSonarImage converter, potentially with some logspace 32bit to linear 8bit conversion. But sonar_image_proc can do that natively now (with the min_db / max_db functionality).

If so, maybe it's vestigial?

@lauralindzey

Copy link
Copy Markdown
Collaborator Author

The sonar postprocessing tools are still a bit messy, in that I use the rqt plugin most of the time, but sometimes I want to see the data in range-theta space rather than euclidean space, so for that I go back to the published drawn_sonar.

I only encountered the code because @micat001 had used it as the starting point for coloring points in sonar_pointcloud.py, and when I asked for an explanation of the calculations he was doing he pointed me here =) And, given that it had confused both of us,

@amarburg

Copy link
Copy Markdown
Collaborator

Hm, my memory of the history is fuzzy but I am wondering if this was a bandaid to experiment with 32-bit logspace rendering before fully integrating the functionality into the core sonar_image_proc library. If it isn't doing anyone any good, maybe change this PR to "Remove postprocess"?

@lauralindzey

lauralindzey commented Feb 4, 2023

Copy link
Copy Markdown
Collaborator Author

I just checked, and my launch files for visualizing sonar data still seem to depend on these nodelets. So, removing them will be a bigger cleanup task.

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.

2 participants