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

Replace memset() in PJONDefines.h to avoid build warning (#1527) #1528

Conversation

virtual-maker
Copy link
Contributor

Fix for Issue #1527

This is a replacement for the memset(&info, 0, sizeof info) line, which causes warnings and fails the Toll Gate. Solution is to use info = PJON_Packet_Info{}; instead.

This solution is taken from the latest PJON version at:
https://github.com/gioblu/PJON/blob/master/src/PJONDefines.h#L398
Latest commit 1be4b15

@mfalkvidd
Copy link
Member

Should (can?) we update the entire Pjon library from upstream (instead of this single change)?

@virtual-maker
Copy link
Contributor Author

Should (can?) we update the entire Pjon library from upstream (instead of this single change)?

@mfalkvidd Hi Mikael,
Yes, upgrading the PJON Library is a good idea and should be done. And yes, the upgrade can potentially introduce new problems.

I personally don't use MySensors with PJON. So I can't judge if the new version really works with MySensors. So I think a PR with the upgrade to the latest PJON version should be requested by a PJON user who can test this with his use case.

@gryzli133 is it possible for you to create a PR with an update to the latest PJON version?

I personally find the CAN bus extension of @AdamSlowik PR #1488 very interesting and would be happy if it will be included in MySensors.

Unfortunately Toll Gate does not accept the pull request as described. And also several other PRs, e.g. @Tico06 with PR #1520, currently have the same problem and are blocked.

Therefore, my suggestion is to first merge this PR which (hopefully) do not changes the current PJON functionality, so that the other PRs are no longer blocked. Afterwards, the PR of the PJON Library upgrade can be placed and tested at leisure.

BR Immo

@mfalkvidd mfalkvidd merged commit 3f37f8d into mysensors:development Jul 6, 2022
@Tico06
Copy link
Contributor

Tico06 commented Jul 7, 2022

Hi There,

Thanks a lot @virtual-maker for the fix and @mfalkvidd for the merge. I just commited let's see what will happen.

Thx and br

Eric.

@Tico06 Tico06 mentioned this pull request Jul 7, 2022
tekka007 pushed a commit to tekka007/MySensors that referenced this pull request Jul 25, 2022
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