Skip to content

Add support for multi-byte pathing#8

Merged
michaelhart merged 4 commits intomichaelhart:mainfrom
jkingsman:add-multibyte-support
Mar 24, 2026
Merged

Add support for multi-byte pathing#8
michaelhart merged 4 commits intomichaelhart:mainfrom
jkingsman:add-multibyte-support

Conversation

@jkingsman
Copy link
Copy Markdown
Contributor

Hello! Thanks for such a lovely library! It powers some great flows in some tools I've made.

This is a minimal case (and, unfortunately, minimally tested, because I don't have access to many multibyte path packets) to add multi-byte pathing support to meshcore-decoder (this would close #5, and make this lib firmware 1.14 ready). All tests pass, including the new ones with actual from-the-message packets (again, only a couple group-messages; would love to add more if you have any).

This was written with LLM assistance, and it did what it tends to do of leaving little tidbits of improvements of error messages etc. as it goes. This may be welcome, or feel free to reject in favor of a more minimalist patch (or no patch at all, if this isn't something you plan to support).

@jkingsman
Copy link
Copy Markdown
Contributor Author

As an update: I've used a fork with this commit in my project and have validated that it behaves reasonably.

@jkingsman
Copy link
Copy Markdown
Contributor Author

jkingsman commented Mar 9, 2026

Popped an additional commit on there with another issue I spotted on traces.

I've also published my fork at https://www.npmjs.com/package/meshcore-decoder-multibyte-patch so I can use it in my app -- I'm hoping that this is temporary, so haven't done any adjustment of licensing. If I don't hear from you in a week or so, I'll tidy up my fork and move the licensing around (respecting your primary license, of course); I don't want to misattribute work to you or misrepresent things but wasn't going to be bothered if this was just a fork-patch for a few days/weeks.

Let me know your thoughts, or if this is something you're not interested in patching right now and I should formalize my fork.

Co-authored-by: Sam Schlegel <git@lutin.us>
@michaelhart
Copy link
Copy Markdown
Owner

Hi @jkingsman -- thank you for this PR! 🙏 I am very sorry to get to this a bit late.

I think it's great as is; the 1 thing I think I'd change is that pathHashSize is undefined for mode 0. Normally I'd provide feedback, but I'm going to merge it as is and quickly change it. I think it's better to always return 1, 2 or 3 consistent with the MeshCore firmware getPathHashSize() which returns one of these 3 values unconditionally. It also saves consumers from needing to frequently do pathHashSize ?? 1 or such to themselves normalize it into a consistent value.

Otherwise, it looks good to me and stood up to scrutiny of Opus :)

I'll try to get this into the analyzer as well ASAP. Thank you!

@michaelhart michaelhart merged commit e2e0ec1 into michaelhart:main Mar 24, 2026
@jkingsman
Copy link
Copy Markdown
Contributor Author

Hey no worries at all -- I know you've been busy! Juan filled me in (congrats!); we do search and rescue together.

I'll do a followup PR to patch up the 1 instead of undefined for the pathHashSize.

@michaelhart
Copy link
Copy Markdown
Owner

Hey no worries at all -- I know you've been busy! Juan filled me in (congrats!); we do search and rescue together.

I'll do a followup PR to patch up the 1 instead of undefined for the pathHashSize.

It's already done! 😁 a9bcc37

@jkingsman
Copy link
Copy Markdown
Contributor Author

Ey awesome!

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.

Add support for multi-byte trace packet types

3 participants