Add support for multi-byte pathing#8
Conversation
|
As an update: I've used a fork with this commit in my project and have validated that it behaves reasonably. |
|
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>
|
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 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! |
|
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 |
|
Ey awesome! |
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).