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

Allow streaming of large messages #568

Merged
merged 5 commits into from
Nov 25, 2019
Merged

Conversation

turgu1
Copy link
Contributor

@turgu1 turgu1 commented Jan 28, 2019

These changes are required to allow for the transmission of large messages through a connected stream. The changes do not have an impact on the class interface and habitual behavior. In particular, it will enable the use of OTA through a stream hooked through the setStream() class method. I've designed such a stream to demonstrate the functionality: https://github.com/turgu1/mqtt_ota_example.git

The change allows readPacket to get very large messages. At the moment, it is limited to 64Kb messages. Transmitting larger messages would allow to send firmware binary updates through the mosquitto_pub command or any other mean as one single message. The change is limited to the readPacket method. The running length variable is transformed to be an int32 instead of an int16.

Guy

These changes are required to allow for the transmission of large messages through a connected stream. The changes do not have an impact on the class interface and habitual behavior. In particular, it will enable the use of OTA through a stream hooked through the setStream() class method. I've designed such a stream to demonstrate the functionality: https://github.com/turgu1/mqtt_ota_example.git

Guy
@knolleary
Copy link
Owner

Hi, could you explain a bit more about why the pr is necessary and what changes actually do?

You may not have noticed, but the library includes unit tests that this change has broken.

@turgu1
Copy link
Contributor Author

turgu1 commented Jan 28, 2019

My call!! sorry about that, I've omitted the one line change in the .h file such that the tests would pass...

@turgu1
Copy link
Contributor Author

turgu1 commented Jan 28, 2019

After my corrective action, testing failed again. The current fail is due to the change that allow for big messages. I thing this is normal because of the change I made.

@turgu1
Copy link
Contributor Author

turgu1 commented Jan 29, 2019

Hello. I modified the test case on oversized messages to get it pass.

@knolleary knolleary merged commit cff1fc7 into knolleary:master Nov 25, 2019
@knolleary
Copy link
Owner

Thanks for the fix - it's appreciated.

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