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

fix parsing trailing message content from CMGR command #182

Conversation

dragonnn
Copy link
Contributor

I think I have here something for issue #180 that doesn't fell like a huge hack.
Still probably a hack but a hope a small one.
It misuses fields attribute from deserialize_struct(), saves it count in the Deserializer.
Then SeqAccess does count how many fields it already parsed, if it hits the branch that no , are present anymore and the count of parsed fields is exactly one less then the expected count of fields to parse it sets a flag in the Deserializer is_trailing_parsing to indicate that parse_str or parse_bytes should now just take everything left and try to parse it.
This does seam to work fine for my use case, but I bet it still has some edge cases where it might not work.
Doesn't seam to break any other tests and I add a test for my use case.

Copy link
Member

@MathiasKoch MathiasKoch left a comment

Choose a reason for hiding this comment

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

I think this looks reasonable..
Perhaps @rmja wants to lend an extra set of eyes here? 👀

@rmja
Copy link
Contributor

rmja commented Nov 22, 2023

I will run it through my sim900 driver test suite and get back.

@rmja
Copy link
Contributor

rmja commented Nov 23, 2023

This change is fine with me, all tests pass fine.

@MathiasKoch MathiasKoch merged commit 0f60a61 into FactbirdHQ:master Nov 23, 2023
13 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