🐛ADI mutex fix for pros 3

[R11G]

Summary:

ADI zero indexing fixes for pros 3

Motivation:

Include ADI mutex fixes onto pros 3

References:

#631 #633

[noam987]

LGTM

[WillXuCodes]

While I wanted this to be a fix #440 for a while, there's a reason why a lot of these have a - 1 and some don't. The situation behind this is slightly more complex than expected. Please take a closer look and hardware test before merging.

[WillXuCodes]

The reason why this is the case is because there were around 3 generations of PROS devs working on this, and it got messy really quick by the time we came around for the ext_adi stuff.

To show you what I mean, look at ext_adi_gyro_init:

ext_adi_gyro_t ext_adi_gyro_init(uint8_t smart_port, uint8_t adi_port, double multiplier) {
	transform_adi_port(adi_port);
	claim_port_i(smart_port - 1, E_DEVICE_ADI);

	if (multiplier == 0) multiplier = 1;
	adi_data_s_t* const adi_data = &((adi_data_s_t*)(device->pad))[adi_port];
	adi_data->gyro_data.multiplier = multiplier;
	adi_data->gyro_data.tare_value = 0;

	adi_port_config_e_t config = (adi_port_config_e_t)vexDeviceAdiPortConfigGet(device->device_info, adi_port);
	if (config == E_ADI_LEGACY_GYRO) {
		// Port has already been calibrated, no need to do that again
		return_port(smart_port - 1, merge_adi_ports(smart_port - 1, adi_port + 1));
	}

	vexDeviceAdiPortConfigSet(device->device_info, adi_port, E_ADI_LEGACY_GYRO);
	if (xTaskGetSchedulerState() == taskSCHEDULER_RUNNING) {
		// If the scheduler is currently running (meaning that this is not called
		// from a global constructor, for example) then delay for the duration of
		// the calibration time in VexOS.
		delay(GYRO_CALIBRATION_TIME);
	}
	return_port(smart_port - 1, merge_adi_ports(smart_port - 1, adi_port + 1));
}

In an effort to prevent the -1 decrement on the smart port and +1 for the adi index every time we do a function call, the smart port is stored as its 0 indexed value and does not need to be -1 in other function calls. This is the pattern across all adi smart+adi port type functions, and is done as a performance optimization (albiet a minor one). The neutral fix to this would be to add a comment as to why there isn't a - 1 on the port claims, but I think there could also be a discussion about if this is worth doing in the first place.

[noam987]

The reason why this is the case is because there were around 3 generations of PROS devs working on this, and it got messy really quick by the time we came around for the ext_adi stuff.

To show you what I mean, look at ext_adi_gyro_init:

ext_adi_gyro_t ext_adi_gyro_init(uint8_t smart_port, uint8_t adi_port, double multiplier) {
	transform_adi_port(adi_port);
	claim_port_i(smart_port - 1, E_DEVICE_ADI);

	if (multiplier == 0) multiplier = 1;
	adi_data_s_t* const adi_data = &((adi_data_s_t*)(device->pad))[adi_port];
	adi_data->gyro_data.multiplier = multiplier;
	adi_data->gyro_data.tare_value = 0;

	adi_port_config_e_t config = (adi_port_config_e_t)vexDeviceAdiPortConfigGet(device->device_info, adi_port);
	if (config == E_ADI_LEGACY_GYRO) {
		// Port has already been calibrated, no need to do that again
		return_port(smart_port - 1, merge_adi_ports(smart_port - 1, adi_port + 1));
	}

	vexDeviceAdiPortConfigSet(device->device_info, adi_port, E_ADI_LEGACY_GYRO);
	if (xTaskGetSchedulerState() == taskSCHEDULER_RUNNING) {
		// If the scheduler is currently running (meaning that this is not called
		// from a global constructor, for example) then delay for the duration of
		// the calibration time in VexOS.
		delay(GYRO_CALIBRATION_TIME);
	}
	return_port(smart_port - 1, merge_adi_ports(smart_port - 1, adi_port + 1));
}

In an effort to prevent the -1 decrement on the smart port and +1 for the adi index every time we do a function call, the smart port is stored as its 0 indexed value and does not need to be -1 in other function calls. This is the pattern across all adi smart+adi port type functions, and is done as a performance optimization (albiet a minor one). The neutral fix to this would be to add a comment as to why there isn't a - 1 on the port claims, but I think there could also be a discussion about if this is worth doing in the first place.

I see what you mean. I just hardware tested a bunch of things including this, and you are right we are claiming the wrong mutex. My preferred fix for this would be to remove the -1 from the return value of the init functions and do a -1 in all the other functions to keep it consistent with the rest of the kernel (particularly the non-ADI devices). I did some hardware benchmarking and found that even with the -1 in the other functions we are able to process 1600 calls to functions such as ext_adi_ultrasonic_get per millisecond. Thus i don't think performance is a concern.

[noam987]

LGTM

Addressed your concerns

[noam987]

LGTM