Boolean INI values are parsed inconsistently

[Sigma1912]

The values for boolean INI variables are not handled consistently in LinuxCNC.

As per documentation
https://linuxcnc.org/docs/devel/html/config/ini-config.html#sub:ini:variables

Boolean values can be on one of TRUE, YES or 1 for true/enabled and one of FALSE, NO or 0 for false/disabled. The case is ignored.

There are some references in the docs indicating that the above does not apply to all boolean variables.
Examples:

LATHE = 1 - Any non-empty value (including "0") causes axis to use "lathe mode" with a top view and with Radius and Diameter on the DRO.

BACK_TOOL_LATHE = 1 - Any non-empty value (including "0") causes axis to use "back tool lathe mode" with inverted X axis.

FOAM = 1 - Any non-empty value (including "0") causes axis to change the display for foam-cutter mode.

Note though that some parts of the code follow none of the above:

linuxcnc/src/emc/usr_intf/gremlin/gremlin.py

Lines 197 to 199 in 878f171

	temp = inifile.find("DISPLAY", "LATHE")
	self.lathe_option = bool(temp == "1" or temp == "True" or temp == "true" )
	self.foam_option = bool(inifile.find("DISPLAY", "FOAM"))

Using ''WRAPPED_ROTARY' with AxisGUI serves as an example of what kind of issues this can lead to:

1. '#'WRAPPED_ROTARY = 1' (or simply no entry)
   -> Interpreter: not wrapped_rotary
   -> AxisGUI : not wrapped_rotary
   MDI: 'G0 A400' -> A-axis moves to '400'

2. 'WRAPPED_ROTARY = 1'
   -> Interpreter: wrapped_rotary
   -> AxisGUI : wrapped_rotary
   MDI: 'G0 A400' -> We get the expected interpreter error and no movement

3. 'WRAPPED_ROTARY = 0' (actually anything other than '1')
   -> Interpreter: not wrapped_rotary
   -> AxisGUI : wrapped_rotary
   MDI command : 'G0 A400' -> We get no error and movement to A: 40

Testing done using 'configs/sim/axis/axis_9axis.ini' in current 2.9 and master

[andypugh]

There certainly are discrepancies.
Taking the example of [DISPLAY]LATHE.

Axis:
LATHE = 1 -> lathe
LATHE = 0 -> lathe
# LATHE = 1 -> not lathe

Gscreen:
LATHE = 1 -> lathe
LATHE = YES -> not lathe

The best way to handle this would be to move the boolean parsing into ini_find, with a method to specify that the returned value should be parsed as a boolean.

[Sigma1912]

Unfortunately it's not just the python code, it's also inconsistent internally:

' WRAPPED_ROTARY = YES' -> not wrapped_rotary

but
'HOME_USE_INDEX = YES' -> homing uses index

[Sigma1912]

Two examples with error output:

'NO_FORCE_HOMING' seems to only accept '1' or '0', anything else defaults to '0' and will output an error message to terminal (the config still starts):
invalid [TRAJ] NO_FORCE_HOMING in /home/user/git/mylinuxcnc-master/configs/sim/axis/axis_9axis.ini (false); using default 0

'ARC_BLEND_FALLBACK_ENABLE' same as above but config does NOT start:
INIFILE: ERR_CONVERSION, section=TRAJ, tag=ARC_BLEND_FALLBACK_ENABLE, num=1, lineNo=64

[BsAtHome]

Not even the documentation can do it right...
docs/src/config/ini-config.adoc:

line 115:
.Boolean Variables
Boolean values can be on one of `TRUE`, `YES` or `1` for true/enabled and one of `FALSE`, `NO` or `0`
for false/disabled. The case is ignored.
...
line 254:
* `DISABLE_CONE_SCALING = TRUE` - Any non-empty value (including "0") will override...
...
line 292:
* `HOMING_PROMPT = TRUE` - Any non-empty value (including "0") will enable showing a prompt...

There needs to be an extra function in C/C++/Py to get Booleans from the ini file that parses the values according to the rules set out and can have a default. Then it needs to be used everywhere.

[Sigma1912]

The function is there:

linuxcnc/src/libnml/inifile/inifile.cc

Lines 82 to 115 in f49e624

	IniFile::ErrorCode
	IniFile::Find(int *result, StrIntPair *pPair,
	const char *tag, const char *section, int num, int *lineno)
	{
	auto pStr = Find(tag, section, num);
	if(!pStr){
	// We really need an ErrorCode return from Find() and should be passing
	// in a buffer. Just pick a suitable ErrorCode for now.
	if (lineno)
	*lineno = 0;
	return(ERR_TAG_NOT_FOUND);
	}
	int tmp;
	if(sscanf(pStr->c_str(), "%i", &tmp) == 1){
	*result = tmp;
	if (lineno)
	*lineno = lineNo;
	return(ERR_NONE);
	}
	while(pPair->pStr != NULL){
	if(strcasecmp(pStr->c_str(), pPair->pStr) == 0){
	*result = pPair->value;
	if (lineno)
	*lineno = lineNo;
	return(ERR_NONE);
	}
	pPair++;
	}
	ThrowException(ERR_CONVERSION);
	return(ERR_CONVERSION);
	}

using this struct which is consistent with the documentation:

linuxcnc/src/emc/ini/emcIniFile.cc

Lines 50 to 71 in f49e624

	IniFile::StrIntPair EmcIniFile::boolMap[] = {
	{"TRUE", 1},
	{"YES", 1},
	{"1", 1},
	{"FALSE", 0},
	{"NO", 0},
	{"0", 0},
	{ NULL, 0 },
	};
	EmcIniFile::ErrorCode
	EmcIniFile::Find(bool *result, const char *tag, const char *section, int num)
	{
	ErrorCode errCode;
	int value;
	if((errCode = IniFile::Find(&value, boolMap, tag,section,num)) == ERR_NONE){
	*result = (bool)value;
	}
	return(errCode);
	}

but it seems that it is only used for boolean entries in the [JOINT_n] sections:

linuxcnc/src/emc/ini/inijoint.cc

Lines 50 to 51 in f49e624

	HOME_USE_INDEX <bool> use index pulse when homing
	HOME_IGNORE_LIMITS <bool> ignore limit switches when homing

In any case there are so many 'Find' functions for ini entries it makes my head spin.

[BsAtHome]

I'm already working on streamlining this problem starting at the bottom.

The real problem is that much of this code is wrongly layered and has duplication. That is in professional circles known as "a complete and utter mess" ;-)