Skip to content
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.

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

Add ParseTagBlock func #112

Merged
merged 1 commit into from
Jul 10, 2024
Merged

Add ParseTagBlock func #112

merged 1 commit into from
Jul 10, 2024

Conversation

icholy
Copy link
Collaborator

@icholy icholy commented Jul 6, 2024

Give users direct access to tag block parsing logic.

Motivation: #111

@icholy icholy mentioned this pull request Jul 6, 2024
@icholy icholy requested a review from aldas July 6, 2024 16:28
@icholy icholy changed the title add ParseTagBlock func Add ParseTagBlock func Jul 6, 2024
@icholy icholy force-pushed the parse-tag-block branch from cc422ce to 8f669fa Compare July 6, 2024 20:53
Copy link
Collaborator

@aldas aldas left a comment

Choose a reason for hiding this comment

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

looking this PR alone - it is OK. but #111 (comment) has valid point. That function returns tagblock but there are no "generic" way to assign that tagBlock to sentence if you decide to parse sentence out of rest of line.

	tag, n, _ := nmea.ParseTagBlock(raw)
	if n > 0 {
		raw = raw[n:]
	}
	if strings.HasPrefix(raw, "$AIVDM") { // fix one specific sentence here
		raw = "!" + raw[1:]
	}
	m, _ := nmea.Parse(raw)
	vdm, ok := m.(nmea.VDMVDO)
	if n > 0 {
		vdm.TagBlock = tag // can assign tag to known type but what if I deal many different sentences?
	}

@kimgr
Copy link

kimgr commented Jul 9, 2024

I like this patch too, for what it's worth. It would be helpful to me to be able to parse only the tag block.

@icholy icholy requested a review from adrianmo July 9, 2024 13:28
@icholy
Copy link
Collaborator Author

icholy commented Jul 10, 2024

@aldas I need a review

@icholy icholy merged commit 65fb477 into master Jul 10, 2024
4 checks passed
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