Closed ethanjli closed 3 years ago
This pull request introduces 1 alert when merging 919b607bd4e5287b15127d0fe9157860705c2e46 into 1c94689b27570c4f5227d27e07c05059269bd54a - view on LGTM.com
new alerts:
This pull request introduces 1 alert when merging d4b73bf22a566ad4eac3209631a498ffb57ef80d into 1c94689b27570c4f5227d27e07c05059269bd54a - view on LGTM.com
new alerts:
This pull request introduces 1 alert when merging a77d64ae70769aabf0eed931c7918c1d3e57115f into 1c94689b27570c4f5227d27e07c05059269bd54a - view on LGTM.com
new alerts:
This pull request introduces 1 alert when merging 7cef74957616dcf913ffb6b0f5f537ee5d3f02e7 into 1c94689b27570c4f5227d27e07c05059269bd54a - view on LGTM.com
new alerts:
This pull request introduces 1 alert when merging 1a07edb25d877903c29ca3d00b39e911b8e5b9bc into 1c94689b27570c4f5227d27e07c05059269bd54a - view on LGTM.com
new alerts:
For records-keeping:
Core/Inc
directory, along with its license file. The PFR library is licensed under the Boost Software License, which is a permissive license and is compatible with our project's licenses. I've recorded the use of this library, along with the license, on our Notion page for tracking third-party works and licenses.@rohanpurohit What level of review and/or testing do you think we should apply to this PR? Do you think we should include this in milestone v0.12, or move it to v0.13?
@rohanpurohit What level of review and/or testing do you think we should apply to this PR? Do you think we should include this in milestone v0.12, or move it to v0.13?
It looks like I can only do light testing on this one! I assume if we go ahead with #403 we'll need to merge this too? I feel like maybe we can go ahead with it if it looks good to you on your end? sorry about not responding sooner.
Great, thanks! I had forgotten to test the alarm muting to get a sense of the latency improvements for that.
This PR is a mostly one-to-one direct port of the backend's implementation of the Event Synchronization protocol from #403, but with some key differences:
==
operator for structs, I had to add the Boost PFR library (documentation here) so that we can compare two instances of structs likeAlarmLimits
. It relies on template metaprogramming magic, but it only compares the literal contents of two instances of a struct and it doesn't seem to work for us in trying to compare the contents of arrays. So we have to define custom==
operators forNextLogEvents
andActiveLogEvents
. The PFR library is third-party code licensed under the Boost Software License, which is fine for our project; I'll provide some more information in the "questions for records-keeping" checklist. Due to the way nanopb works, until we address #405 we have a templated==
operator in the global namespace, which means you might be able to erroneously use==
and!=
on types which shouldn't have such an operator, or else you might see cryptic compiler errors.None
from a sender, in C++ we have to return aStateOutputStatus::none
error code.