A variable not updated in ISR

Go To Last Post
29 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I am reading an analog value using ADC from multiple pins and I would like to see if I would get more correct readings if I were to ignore several readings after switching an input source pin for ADC.

 

The device is ATmega32A running at 8 MHz.

 

Here's my code:

#include "temperature.h"

#include <avr/interrupt.h>
#include <avr/io.h>

#include "io_helper.h"

#define SENSOR_COUNT                  4
#define REFERENCE_VOLTAGE             2.56 // In volts.
#define SMOOTHING_FACTOR              0.0f

#define TEMPERATURE_PORT              A
#define TEMPERATURE_BOTTOM_LEFT_PIN   0
#define TEMPERATURE_BOTTOM_RIGHT_PIN  1
#define TEMPERATURE_BOTTOM_MIDDLE     2
#define TEMPERATURE_BOTTOM_TOP        3

const uint8_t pin_ids_[] =
{
    TEMPERATURE_BOTTOM_LEFT_PIN,
    TEMPERATURE_BOTTOM_RIGHT_PIN,
    TEMPERATURE_BOTTOM_MIDDLE,
    TEMPERATURE_BOTTOM_TOP
};

uint8_t active_pin_;
volatile uint8_t counter_ = 0;

void temperature_init()
{
    active_pin_ = 0;
    counter_ = 0;

    // Set input direction.
    IO_DIRECTION_IN(TEMPERATURE_PORT, TEMPERATURE_BOTTOM_LEFT_PIN);
    IO_DIRECTION_IN(TEMPERATURE_PORT, TEMPERATURE_BOTTOM_RIGHT_PIN);
    IO_DIRECTION_IN(TEMPERATURE_PORT, TEMPERATURE_BOTTOM_MIDDLE);
    IO_DIRECTION_IN(TEMPERATURE_PORT, TEMPERATURE_BOTTOM_TOP);

    // Disable internal pull-ups.
    INT_PULLUP_DISABLE(TEMPERATURE_PORT, TEMPERATURE_BOTTOM_LEFT_PIN);
    INT_PULLUP_DISABLE(TEMPERATURE_PORT, TEMPERATURE_BOTTOM_RIGHT_PIN);
    INT_PULLUP_DISABLE(TEMPERATURE_PORT, TEMPERATURE_BOTTOM_MIDDLE);
    INT_PULLUP_DISABLE(TEMPERATURE_PORT, TEMPERATURE_BOTTOM_TOP);

    // Set up ADC.
    ADMUX = (1 << REFS1) | (1 << REFS0); // Internal 2.56 V reference.
    ADCSRA |= (1 << ADIE); // Enable interrupt.
    sei();
    ADCSRA |= (1 << ADPS2) | (1 << ADPS1) | (1 << ADPS0); // 128 prescaler.
    ADCSRA |= (1 << ADEN); // Enable ADC.
    ADCSRA |= (1 << ADSC); // Start conversion.
}

ISR(ADC_vect)
{
    // Skip 10 readings.
    // The following line for debugging purposes.
    temperature.values[pin_ids_[active_pin_]] = counter_;
    if (++counter_ < 10) return;
    // The program never reaches this point.
    // The following line for debugging purposes.
    PORTD |= (1 << PORTD7);

    // Read temperature.
    temperature.values[pin_ids_[active_pin_]] =
        temperature.values[pin_ids_[active_pin_]] * SMOOTHING_FACTOR +
        REFERENCE_VOLTAGE * ((float)ADC / 1024) * 100 * (1 - SMOOTHING_FACTOR);

    // Next input pin.
    counter_ = 0;
    active_pin_++;
    if (active_pin_ >= SENSOR_COUNT) active_pin_ = 0;
    ADMUX &= 0b11100000; // Clear MUXn bits.
    ADMUX |= pin_ids_[active_pin_];
    ADCSRA |= (1 << ADSC); // Start next conversion.
}

I have a helper class for pin setting macros (should not be relevant).

/*
 * io_helper.h
 */ 

#ifndef IO_HELPER_H_
#define IO_HELPER_H_

#define CONCAT2(x, y) x ## y
#define CONCAT3(x, y, z) x ## y ## z
#define DDRx(port, pin)  CONCAT2(DDR,  port) &= ~(1 << CONCAT3(DD,   port, pin))

#define PORTx_ON(port, pin)  CONCAT2(PORT, port) |=  (1 << CONCAT3(PORT, port, pin))
#define PORTx_OFF(port, pin) CONCAT2(PORT, port) &= ~(1 << CONCAT3(PORT, port, pin))

#define IO_DIRECTION_IN(port, pin)  CONCAT2(DDR, port) &= ~(1 << CONCAT3(DD, port, pin))
#define IO_DIRECTION_OUT(port, pin) CONCAT2(DDR, port) |=  (1 << CONCAT3(DD, port, pin))

#define INT_PULLUP_ENABLE(port, pin)  PORTx_ON(port, pin)
#define INT_PULLUP_DISABLE(port, pin) PORTx_OFF(port, pin)

#endif /* IO_HELPER_H_ */

The problem is that variable counter_ always has a value of 1. I have a HD44780 display connected, so I can see the content by setting a temperature variable to the value of counter_ for debugging purposes.

 

The code inside ISR after the check is never reached (see the comment).

 

I have marked the variable as volatile, the ISR should be forced to read it and update it correctly, as I understand. The variable is not accessed from any other point in the program, except in temperature_init() procedure which is called only once after startup.

 

What am I doing wrong?

This topic has a solution.
Last Edited: Tue. Dec 19, 2017 - 10:38 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

I was going to say "volatile" but you have that so let's see the code that accesses the variable and puts it on the LCD

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I would prefer not to do that if not needed, because:

  • I am not accessing counter_ externally anywhere.
  • The display is working with temperature variable which does not have anything to do with the counter itself.
  • The display is correctly displaying any value that I force to it.
  • Display's compilation unit is fairly large with menu and button handling and what not, you would spend too much time in it for, I think, no benefit, because of the points mentioned above.

 

Let me also point out that I have an LED connected to PD7. I am quite sure there is no problem with it, because:

  • It blinks twice at startup (so I am sure it works).
  • It is not lit up at any other place in the code (I am 100% certain of this).
  • If I move the line PORTD |= (1 << PORTD7) just above the check, it lights up.

 

Also note that I am not including temperature.c (the file containing the original piece of code) anywhere in my other units.

 

Therefore, regardless of my display code correctness, there should be something quirky about this specific unit.

 

Unfortunately, I have no means to communicate over serial at the moment, so cannot check out the value of the counter in that way.

Last Edited: Sun. Dec 17, 2017 - 03:06 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

Why are you putting trailing underscores on names? I don't think it's relevant, it just seems like a Bad Idea in general. (Don't put underscores on the ends of names unless you're a compiler dev. Really, just don't. The actual rules are complicated enough that you'll screw them up and be sad. Just don't do it.)

 

If I were gonna guess, I'd guess that you're accidentally calling temperature_init more than once, or that something is overrunning a buffer that happens to be near the counter.

 

If nothing else accesses the variable, you should probably declare it static. This won't matter, I hope, but it's good practice.

This reply has been marked as the solution. 
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

I don't have a datasheet nearby, so I can't check for myself. Is the ADC set up for free-running mode, or just a single conversion?

My guess would be that it is making one conversion (hence counter_ is 1), but then you return from the ISR early and never starts another conversion.

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Svuppe wrote:

My guess would be that it is making one conversion (hence counter_ is 1), but then you return from the ISR early and never starts another conversion.

Of course. As soon as I've read your post, I facepalmed at myself. It's running in single conversion mode, so the next interrupt was never called. Calling ADCSRA |= (1 << ADSC); before returning solves the problem.

 

Thanks for pointing out.

the_real_seebs wrote:

Why are you putting trailing underscores on names? I don't think it's relevant, it just seems like a Bad Idea in general.

It's just a naming convention I use for private variables, coming from my C++ background. Now you got me curious, but I cannot find any references discouraging the use of underscores.

 

Can you explain why it is bad or point out to some references? Thanks.

Last Edited: Sun. Dec 17, 2017 - 06:39 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

Historical practice in C was to use leading underscores for implementation internals. As a result, there's reserved space there; anything starting with an underscore followed by an uppercase letter is reserved, for instance.

 

But that's for the implementation. If you are not the implementation, you're not supposed to use those names.

 

 

In C++, if you want something to be private, declare it as private. Or use static, if you want to make something not-global.

 

Here's the problem: Say you do this. And say you have another module, which also does this. So you have two modules both of which say

 

uint8_t counter_;

 

well, now you have a problem: those are both global variables, with the same name. And you now have a single variable named "counter_" which is used in two places.

 

If the trailing underscore makes you feel in any way like you've done something to make a thing private or internal, it has lied to you. And that is a reason not to do it. If you want "static", use "static".

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

Witchunter wrote:
It's just a naming convention I use for private variables, coming from my C++ background. Now you got me curious, but I cannot find any references discouraging the use of underscores.   Can you explain why it is bad or point out to some references? Thanks.

 

Yes, the ending_ underscore is one of several conventions that different part of the C++ community uses for private class member variables (one of them, of-course, is to not mark them at all in any special way). In C this is not at all an established convention. (Modern IDEs and editors has largely removed at least one reason for such conventions.)

 

Don't expect all C coders to know C++ (especially when it comes to one of several competing conventions). 

 

If you want the widest possible audience for your C code then don't use the convention in C.

 

EDIT: And of-course the_real_seebs has pointed out that you're trying to mimick C++ (private) member variables in C without actually achieving any of the semantics of them.

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

Last Edited: Sun. Dec 17, 2017 - 07:01 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

JohanEkdahl wrote:

EDIT: And of-course the_real_seebs has pointed out that you're trying to mimick C++ (private) member variables in C without actually achieving any of the semantics of them.

I am aware that C does not have a notion of private members as C++ does. I was not trying to mimick the feature, it was purely a naming habit that I have from C++ world.

 

Nonetheless, I can see the reasoning from both you and @the_real_seebs and why I should not practice it in C code. I'll have to force myself to push some C++ conventions out of my head when working with C.

 

Thank you both for the advice.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

Witchunter wrote:
and why I should not practice it in C code.

Remember that conventions are conventions.  You may use whatever style you prefer.  Regardless of the posts, there is no such thing as style police.  Just because a trailing underscore wasn't familiar to a few respondents dos not mean that it is wrong in any fashion. 

 

I suppose they have stron opinions on why any brace style other than theirs is somehow wrong, as well.

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

theusch wrote:
Remember that conventions are conventions.  You may use whatever style you prefer.  Regardless of the posts, there is no such thing as style police.  Just because a trailing underscore wasn't familiar to a few respondents dos not mean that it is wrong in any fashion. 

In any fashion?

 

It definitively is not wrong in a syntactic fashion.

 

But for communicating with other people - which is what conventions are often about, I argue it is bad. I win't use the word "wrong" for that, since it has a "binary" meaning.)

 

If the OP wants to keep his code to himself he can do whatever he wants. If the OP wants to discuss his code with others then he can still do what he wants but the advice I was trying to give was simply that such communication would be easier if he didn't adopt conventions that are likely less known to the community of C coders.

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Code style isn't always a right/wrong thing, but it's worth noting that it does have an impact on maintenance and development over time.

 

Brace style is mostly like which side of the road you drive on; it doesn't matter much what the answer is, but it's pretty important that everyone agrees (at least within a given code base).

 

In general, a lot of the code I see targeting AVR systems is... frankly sort of atrocious. It's worth being aware that code quality is absolutely a thing, and it will affect maintainability, and often also performance. (I have repeatedly found that code in widely-used AVR libraries in the Arduino side of the world is painfully slow. Doubling performance is often pretty achieveable.)

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1
Deleted
Last Edited: Mon. Dec 18, 2017 - 06:25 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

I personally see nothing wrong with an underscore used as a SUFFIX. The one thing I would caution against is using it as a PREFIX as a single underscore prefix denotes the C library namespace and double is the compiler's own namespace. 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Based on all of your comments, I would like to ask you the following.

 

Say, that I have a header file that I include in other compilation units that require it defined as:

/*
 * whatever.h
 */ 

#ifndef WHATEVER_H_
#define WHATEVER_H_

extern uint8_t temperature;

extern void set_max_temperature(uint8_t max_temperature);

#endif /* WHATEVER_H_ */

And then I have a source file which I don't include anywhere.

/*
 * whatever.c
 */ 

#include "whatever.h"

uint8_t temperature;
static uint8_t max_temperature_;

void set_max_temperature(uint8_t max_temperature)
{
    max_temperature_ = max_temperature;
    
    if (temperature > max_temperature_)
        temperature = max_temperature_;
}

Without trying to start a flame war about naming the stuff, doesn't this sound logical even in C?

  • temperature can be accessed from compilation units that include the header, so is named without an underscore.
  • max_temperature_ cannot be accessed externally, so is named with an underscore. It's clearer what the scope of the variable is this way.
  • Having a name with an underscore also helps differentiate between a member variable and function parameter.

 

Now, I may be heavily biased by C++, but still this seems to be the case where having an underscore at the end adds information and clarity about variable scope. Therefore, I would be encouraged to use this naming style.

 

Again, everybody has personal preferences, but is my reasoning sound? And is there any other disadvantage for this style, except for not conforming to how majority of C community does things?

 

PS. I am aware that temperature can be set without checking for being lower than max temperature. It's just a silly code sample.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Always write code for the maintainer so do whatever will help her understanding of the code. 

 

Another idea is to use case for differentiation. So "locals" might be all lower case. 

 

Another idea is to consider prefixing all exposed/public symbols with a module prefix such as ADC_ or UART_ etc. 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I'll stick my neck out and ask: Have you considered actually coding in C++ rather than C?

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

JohanEkdahl wrote:

I'll stick my neck out and ask: Have you considered actually coding in C++ rather than C?

 

Yes, but I've decided not to for a couple of reasons:

  • It's a great opportunity for me to learn C, and
  • I'll get more support from the community for AVR programming if I am sticking to C (and avoid attacks from people who are bashing C++ in AVR environment with not too much ground, as I've seen in many threads).

 

I've browsed the forum to have an informed decision about language choice for AVR, and seen the pros and cons of each one. C juts seems better fit for me now.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

JohanEkdahl wrote:
if he didn't adopt conventions that are likely less known to the community of C coders.

C'mon.  Are you sayihng that if someone posts a screenful-sized code fragment in C that has e.g. one or more of:

 

-- trailing size/type info vs. the current politically-correct preceding

-- camel-case for variable names indicating words

-- underscore separating words in variable names

-- nothing separating words in variable names

-- requirement for using whole words in variable names ...

-- ... or conversely, using a set of agreed-upon abbreviations to keep variable names to a reasonable size

-- or, even, use a trailing underscore on a variable name to designate something

 

IMO/IME any of the above might be adapted by a programming team.  IMO any experienced coder, including those style-police responding here, should be able to "read past" something as simple as the head-of-a-pin angel count under fire here.  If not, how do you ever expect to meld with code reading from other upbringings?

 

 

 

 

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

Historical practice in C was to use leading underscores for implementation internals. As a result, there's reserved space there; anything starting with an underscore followed by an uppercase letter is reserved, for instance.

 

leading underscores was reserved by the C and C++ standards separately. Though compilers don't care to enforce it.

 

So anyone that was a stickler to standards would just trailing underscores which is fine.

 

Using such notation is simply for being able to read code easier. Nothing to do with false beliefs in private.

Both Microsoft and Google have it in their own development standards to name variables as such. 

It is a holdover from when IDEs weren't as nice as they are now giving you hints on variable declarations as type. But then again it's also a aid for people who don't use IDEs but rather VIM/EMACs which there is many.

Last Edited: Mon. Dec 18, 2017 - 02:22 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

mroszko wrote:
Though compilers don't care to enforce it.

When [e.g. C] language standards were developed/finalized, decisions were made on "breaking existing code".  Sometimes, that ended up to allow legacy constructs.

 

Let's see how my standardese is this week.  From WG14/N1256 Committee Draft — Septermber 7, 2007 ISO/IEC 9899:TC3
 

6.4.2 Identifiers
6.4.2.1 General
Syntax
1 identifier:
identifier-nondigit
identifier identifier-nondigit
identifier digit
identifier-nondigit:
nondigit
universal-character-name
other implementation-defined characters
nondigit: one of
_ a b c d e f g h i j k l m
n o p q r s t u v w x y z
A B C D E F G H I J K L M
N O P Q R S T U V W X Y Z
digit: one of
0 1 2 3 4 5 6 7 8 9
Semantics
2 An identifier is a sequence of nondigit characters (including the underscore _, the
lowercase and uppercase Latin letters, and other characters) and digits, which designates
one or more entities as described in 6.2.1. Lowercase and uppercase letters are distinct.
There is no specific limit on the maximum length of an identifier
 

So the standard will enforce ;) that an identifier can have an underscore. 

 

If it is a leading underscore, then e.g.

— All identifiers that begin with an underscore and either an uppercase letter or another
underscore are always reserved for any use.
— All identifiers that begin with an underscore are always reserved for use as identifiers
with file scope in both the ordinary and tag name spaces.
...

...predefined macro names shall begin with a leading underscore followed by an uppercase letter or a second
underscore.

...

 

...and that is about it.  If OP here wants to use a trailing underscore, or say n consecutive to denote scratchpad or global or whatever, don't go pulling style police.  OP's "group" may have that as a convention.  Just because you haven't seen the convention, or your group uses a different one, is no reason to proselytize.  [How do you think baby conventions are made, anyhow?  Someone did something not seen before; others thought it was a good idea.]

 

From the Rationale

• A strictly conforming program can use only a restricted subset of the identifiers that
begin with underscore (§7.1.3). Identifiers and keywords are distinct (§6.4.1).
Otherwise, programmers can use whatever internal names they wish; a conforming
implementation is guaranteed not to use conflicting names of the form reserved for the
25 programmer. (Note, however, the class of identifiers which are identified in §7.26 as
possible future library names.)

...

Also reserved for the implementor are all external identifiers beginning with an underscore, and
all other identifiers beginning with an underscore followed by a capital letter or an underscore.
This gives a name space for writing the numerous behind-the-scenes non-external macros and
functions a library needs to do its job properly.
With these exceptions, the Standard assures the programmer that all other identifiers are
25 available,

 

<emphasis mine>

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

Last Edited: Mon. Dec 18, 2017 - 03:16 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

What's with the "broadsides", Lee? I was merely trying to say it would be easier communicating. I never said OP should absolutely do or not do anything.

 

Has someone been rude to you lately? You sound bitter..

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

JohanEkdahl wrote:
What's with the "broadsides", Lee? I was merely trying to say it would be easier communicating.

Intended to be targeted, not a broadside.

 

OP is new here.  Op posted reasonable-sized code fragments, and did a reasonably good job presenting the situation for a first-timer.

 

I put you and andy into the category of experienced people that should be able to read right through something as trivial as a trailing underscore.  A mention, perhaps.  A recommendation?  IMO way too strong.

 

Let's say what I posted is a defense of trailing underscore, and claim that you and andy are wrong.  How do y'all feel then?

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

Last Edited: Mon. Dec 18, 2017 - 03:26 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

theusch wrote:
I put you and andy into the category of experienced people that should be able to read right through something as trivial as a trailing underscore.

I won't speak for Andy. He will do that himself if he wants to.

 

I can see through the trailing underscore. Again: I merely pointed out that to facilitate communication with the largest base of C developers it might be worthwhile not using it. Nothing more. Not a big thing. And of-course the OP can do as he wishes. Re "proselytize", that's just ridiculous.

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

JohanEkdahl wrote:

Re "proselytize", that's just ridiculous.

;)  Now I had to re-read the thread, and your responses in particular (interspersed with the others).

 

Ridiculous?

pros·e·lyt·ize

ˈpräs(ə)ləˌtīz/

verb: proselytise; 3rd person present: proselytises; past tense: proselytised; past participle: proselytised; gerund or present participle: proselytising

 

1.  convert or attempt to convert (someone) from one religion, belief, or opinion to another.

2. advocate or promote (a belief or course of action).

 

JohanEkdahl wrote:

... I argue it is bad. ...

... the advice I was trying to give ...asier if he didn't adopt conventions that are likely less known to the community of C coders.

Not so ridiculous after all, is it?  Anyway, I think I've presented what I wanted:  criticism of minor style matters serves nothing but to derail the focus on the real factors affecting OP's situation. 

 

Maybe I seem ornery because I seem to recall similar "style police" rants about tabs/spaces; brace style; type prefixing; and now, horrors, trailing underscore -- all in the past few months.

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Witchunter wrote:

Based on all of your comments, I would like to ask you the following.

 

Say, that I have a header file that I include in other compilation units that require it defined as:

/*
 * whatever.h
 */ 

#ifndef WHATEVER_H_
#define WHATEVER_H_

extern uint8_t temperature;

extern void set_max_temperature(uint8_t max_temperature);

#endif /* WHATEVER_H_ */

And then I have a source file which I don't include anywhere.

/*
 * whatever.c
 */ 

#include "whatever.h"

uint8_t temperature;
static uint8_t max_temperature_;

void set_max_temperature(uint8_t max_temperature)
{
    max_temperature_ = max_temperature;
    
    if (temperature > max_temperature_)
        temperature = max_temperature_;
}

Without trying to start a flame war about naming the stuff, doesn't this sound logical even in C?

  • temperature can be accessed from compilation units that include the header, so is named without an underscore.
  • max_temperature_ cannot be accessed externally, so is named with an underscore. It's clearer what the scope of the variable is this way.
  • Having a name with an underscore also helps differentiate between a member variable and function parameter.

 

 

It's not "clearer" unless everyone else has always used the name that way. And since the example that started this was a non-static variable with a trailing underscore, obviously it's not actually always used that way.

 

 

So it's clear that it's probably intended to mean something, but it's not obvious what, and "static" already communicates the thing.

 

Quote:
Now, I may be heavily biased by C++, but still this seems to be the case where having an underscore at the end adds information and clarity about variable scope. Therefore, I would be encouraged to use this naming style.

 

Again, everybody has personal preferences, but is my reasoning sound? And is there any other disadvantage for this style, except for not conforming to how majority of C community does things?

 

PS. I am aware that temperature can be set without checking for being lower than max temperature. It's just a silly code sample.

 

Your reasoning isn't sound, because it presupposes universal prior knowledge of the convention. Without that, it doesn't work.

 

It also creates the problem that if you decide to change a thing's visibility, you have to go find every instance of it and change the name. That's not ideal either.

 

It's a possible source of errors, and doesn't actually add communicative value.

 

And given how many cases I've seen where problems involved values that should have been static, and weren't, and just relied on some undocumented naming convention as a way to try to be "private"... I think I'd suggest sticking with "static", and using that for private things, and using *only* that. (Or the actual private keyword in C++.)

 

 

YMMV. Yes, as an experienced programmer, I can usually read code even if it's not ideal; I used to be an IOCCC judge. Doesn't mean I shouldn't suggest things likely to improve readability and communication for other users.

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Deleted

Last Edited: Tue. Dec 19, 2017 - 07:54 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

Guys, can we put this one to bed for now? Having not been involved in the discussion, it is not making for good reading by others.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Agree. I'll lock this just to encourage that idea ;-)

Topic locked