<div dir="ltr"><div><div><div><div>I think there are three things worth considering<br><br></div><div>1) Which gives you correct answers.<br><br></div>2) Which is easier to use/maintain<br><br></div>3) Which is most efficient.<br><br></div> The readSomePin();, now read value from somePinStatus variable idea is terrible because it splits the operations. If different threads (including interrupts) call readSomePin() and later (even in the next instruction) you read the extern variable, you have lost the context. This thus fails the "correctness" test because you don't know when the read occurred.<br><br></div><div>Secondly you have to coordinate the use of the read and the variable check. That makes it hard to do things like doing if/while code and requires more code (and thus more chances to mess up).<br><br></div><div>Lastly, it forces the use of memory accesses (can't be registerised) and is therefore potentially slower and uses more memory<br><br></div><div>The read(*valpointer) mechanism is at least better in that the operation is "atomic" and you know when you read the value.<br><br></div><div>However, it still means you have to check the value after reading it.<br><br></div><div>It also forces the use of memory (since you're using a pointer) and is thus potentially slower.<br><br></div><div>By bet would be using the return value where you can. value= readPin();<br><br></div><div>This has all the good stuff we're looking for: <br></div><div>It is "correct"<br></div><div>It is easy to read/maintain.<br></div><div>It is efficient since it can be fully reegisterised.<br><br></div><div>Also if you write code like<br><br></div><div>/* Check for three button salute */<br></div><div>If (button1_pressed() && button2_pressed() && button3_pressed()) ....<br><br></div><div>then only the tests that are required will be performed. If button1 is not pressed, then the if(...) will not check button2 or button3.<br><br><br></div><div>However.... there is never a one-size-fits-all answer and discretion is advised.<br><br></div><div>I severely detest code that returns structures. Use pointer passing for structures.<br><br></div><div>If a function returns a value and maybe also other "out of band" info such as status etc, then consider using a structure for the status stuff and making it optional.<br><br></div><div>eg. <br><br></div><div>int button1_pressed(int *time_since_previous_press)<br>{<br></div><div> if (time_since_previous_press)<br></div><div> *time_since_previous_press = now - last_press_time_stamp;<br></div><div> return current_button)state;<br></div><div>}<br><br><br></div><div>Now you can get that extra data or just call with button1_pressed(NULL) if you don't care.<br><br></div><div>Also, bear in mind the types you are using. On ARM it is generally better to use 32 bit values than 8 bits or 16 bits, but again that's not a hard and fast rule.<br><br></div><div><br><br></div><div><br></div><div><br><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Sat, Feb 6, 2016 at 12:37 AM, Robin Gilks <span dir="ltr"><<a href="mailto:robin@gilks.org" target="_blank">robin@gilks.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
> Good day all,<br>
> What is the opinion of this esteemed group on the best/proper/whatever<br>
> way<br>
> to get the measured value from some external input from a micro in C.<br>
> By measured value I mean something like ADC, OW temp sensor, I2C sensor<br>
> state etc.<br>
> I am considering the following three scenarios in increasing order of<br>
> goodness:<br>
> 1) function returns the measured value indicating error by out of range<br>
> result (i.e 0xFF for 8 bit int)<br>
> i.e:<br>
> ...<br>
> unit8_t readSomePin(void); // 0xFF indicates error<br>
><br>
> 2) function returns the measured value indicating status of measurement in<br>
> another extern. variable<br>
> ...<br>
> extern uint8_t readSomePinStatus;<br>
> unit8_t readSomePin(void); // Status in readSomePinStatus<br>
><br>
> 3) function returns status, the value is stored in an argument variable:<br>
> ...<br>
> unint8_t readSomePin(uint8_t *returnedValue);<br>
><br>
> So, what is best? Is there a better way? This could no doubt also be done<br>
> with Structs, but seems like overkill to me.<br>
<br>
</span>I'd go for option 3 as the function prototype provides all the information<br>
about the size and sign of the parameter as well as the function itself<br>
giving the result of success or failure which will make the code more<br>
maintainable.<br>
<span class="HOEnZb"><font color="#888888"><br>
--<br>
Robin Gilks<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
<br>
<br>
_______________________________________________<br>
Chchrobotics mailing list <a href="mailto:Chchrobotics@lists.linuxnut.co.nz">Chchrobotics@lists.linuxnut.co.nz</a><br>
<a href="http://lists.ourshack.com/mailman/listinfo/chchrobotics" rel="noreferrer" target="_blank">http://lists.ourshack.com/mailman/listinfo/chchrobotics</a><br>
Mail Archives: <a href="http://lists.ourshack.com/pipermail/chchrobotics/" rel="noreferrer" target="_blank">http://lists.ourshack.com/pipermail/chchrobotics/</a><br>
Meetings usually 3rd Monday each month. See <a href="http://kiwibots.org" rel="noreferrer" target="_blank">http://kiwibots.org</a> for venue, directions and dates.<br>
When replying, please edit your Subject line to reflect new subjects.<br>
</div></div></blockquote></div><br></div>