Attention is currently required from: tnt. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-e1d/+/29618 )
Change subject: usb: Add support for new GPSDO status field "Accumulated error" ...................................................................... Patch Set 3: (2 comments) File src/ice1usb_proto.h: https://gerrit.osmocom.org/c/osmo-e1d/+/29618/comment/3fee4994_e0e0b9c2 PS3, Line 68: int16_t err_acc; /*!< Accumulated error */ I think it might make sense to add a comment that this field was added by firmware version `git describe` of the hardware/firmware git repo. File src/usb.c: https://gerrit.osmocom.org/c/osmo-e1d/+/29618/comment/9d5bdce3_cf522d2c PS3, Line 715: "Some values will be zeroed\n", I don't think we want to send one NOTICE every status_cb. It's perfectly legal (and probably normal for most users) to have older firmwares. So if at all, I think we need to make sure this message is only printed once during osmo-e1d execution. Also, the function could benefit from a comment explaining that the len could be shorter in older firmware versions. -- To view, visit https://gerrit.osmocom.org/c/osmo-e1d/+/29618 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-e1d Gerrit-Branch: master Gerrit-Change-Id: I4222bf22267f8343abf1e97546111ceb1c299846 Gerrit-Change-Number: 29618 Gerrit-PatchSet: 3 Gerrit-Owner: tnt <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <[email protected]> Gerrit-Attention: tnt <[email protected]> Gerrit-Comment-Date: Thu, 06 Oct 2022 07:46:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
