When I tested this Qt did not run the helper functions as test functions. It appears to be able to differentiate between the two without any additional directives.
As requested, I've made the helper functions private instead of private slots to make it absolutely clear which functions are helper functions and which aren't.
What do you mean by : "if init() changes, this function will behave differently, but it won't be reflected in the documentation" the function document says the same thing: "returns the return value of the init() method" so if init() changes this function return changes because it's an overloaded method of the init() function.
Absolutely, I'm doing it incrementally and slowly to not to make the code reviews harder and confusing git. I did for so many and this file will get its turn soon.
We had discussed the namespace indentation in another comment and resolved. "This is the namespace brace and the namespaces braces are not indenting the code. so are at the same column as class brace in this case."
I tried to remove it but seems like Jira doesn't listen. sadly after we discussed in a conversation we had that Sarina arranged, when you told me to merge the staging branch into my working branch I didn't envision at that moment Jira doesn't let us remove it after it has been added. Fortunately what we currently can do is to only review files in dialin/squish folder.
If the number of homing was exceeded and we failed, we go back to Idle, but the valve cannot go anywhere. It cannot receive any command to go to any of the positions. When the device turned on, we go straight to homing not started state though.
This code review started > 2 weeks ago and since then the message list document was updated as per Sean's requested changes which I've been working on in a separate branch and story.
The code in this header file should be added to the SystemCommMessages group for now. This header file will be going away at some point and its definitions distributed to other header files.
There are more public functions below this line. I generally prefer public functions on top and private function below, but this is not required and would conflict with other considerations like keeping Dialin functions (which are public) at very bottom of module. Recommend removing this comment.
Yes of course, this is a very old comment made early on during development. See the newer version of this file, guiglobals.h, in the files list to the left where this comment is no longer present. That should match exactly with the branch you checked out.
That is obvious that calling multiple layers of functions on each user slider movement and value change. Let's say we have a slider in range of -100 to +700 (which is a valid actual range in PRS) with resolution of 1 and user easily can drag the slider, then imagine what amount of call will happen for a single user slide. It may not currently visibly show a slow down but later by adding more into the code it may cause us problems. The SRSUI doesn't force the design it's up to designer and developer to meet the requirement while keeps the code performance high. If it's absolutely necessary the please add a release signal and handle it at that time.
If payload is 2 U32s, why is memcpy below only copying one U32? Also, why is payload typed as VALVE_POSITION_T? So I see 3 different versions of what the payload is here.