Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CY8CPROTO-062-4343W - AnalogIn behaving oddly #11378

Closed
nick-atmosphereiot opened this issue Aug 29, 2019 · 10 comments
Closed

CY8CPROTO-062-4343W - AnalogIn behaving oddly #11378

nick-atmosphereiot opened this issue Aug 29, 2019 · 10 comments

Comments

@nick-atmosphereiot
Copy link
Contributor

Description

the read() function for pins P10_0 - P10_3 (Thermistor) is behaving oddly. Instead of receiving values between 0 and 1.0, it's returning values around 4.0:

P10_0: 4.164062                                                                                                                                                                                                           
P10_1: 4.191406                                                                                                                                                                                                           
P10_2: 4.207031                                                                                                                                                                                                           
P10_3: 4.222656  

Here's the code to replicate:

	AnalogIn A_10_0(P10_0);
	AnalogIn A_10_1(P10_1);
	AnalogIn A_10_2(P10_2);
	AnalogIn A_10_3(P10_3);

	while(1)
	{
		printf("P10_0: %f\r\n", A_10_0.read());
		printf("P10_1: %f\r\n", A_10_1.read());
		printf("P10_2: %f\r\n", A_10_2.read());
		printf("P10_3: %f\r\n", A_10_3.read());
		printf("\r\n");
		wait_ms(1000);

	}

Issue request type

[ ] Question
[ ] Enhancement
[X] Bug
@0xc0170
Copy link
Contributor

0xc0170 commented Aug 30, 2019

cc @ARMmbed/team-cypress

@kjbracey
Copy link
Contributor

kjbracey commented Aug 30, 2019

It's being converted from 12-bit to 16-bit twice.

Outer implementation is here:

float analogin_read(analogin_t *obj)
{
return analogin_read_u16(obj) * (1.0f / (1.0f + CYHAL_ADC_MAX_VALUE));
}
uint16_t analogin_read_u16(analogin_t *obj)
{
return cyhal_adc_read_u16(&(obj->hal_adc_channel));
}

and CYHAL_ADC_MAX_VALUE is set to 0xFFF, which is true for the hardware register, but cyhal_adc_read_u16 has already converted to a 0-0xFFFF range. (ish - see below).

analogin_read should be returning analogin_read_u16() * (1.0f / 0xffff).

(Not / 0x10000, like the existing code does).

While here, I'll note that the scaling in cyhal_adc_read_u16 is a little bit off - it ends up just multiplying by 16, so you get an answer between 0x0000 and 0xFFF0. You could never get 1.0 from the float version.

An better and still-efficient way to increase number of bits is to do this, replicating the top nibble:

uint16_t scaled_result = (result << 4) | (result >> 8);

So 0xABC becomes 0xABCA. (And 0x000->0x0000, 0xFFF->0xFFFF).

@kjbracey
Copy link
Contributor

kjbracey commented Aug 30, 2019

(Paranoia compelled me to check that 0xFFFF * (1.0f / 0xFFFF) doesn't end up computing something bigger than 1. It gives exactly 1, thankfully).

Hat tip: http://weitz.de/ieee/

@nick-atmosphereiot
Copy link
Contributor Author

@kjbracey-arm Thanks for the reply, I'll give that a shot and open a PR once I give it a test.

-Nick

@nick-atmosphereiot
Copy link
Contributor Author

@kjbracey-arm Those changes do seem to fix the range such that it's between 0.0 and 1.0, but I'm still having issues getting values out of it that make sense. Maybe I need to dig into the PSOC6 SDK some more.

I've disconnected the Thermistor snap-off panel from my board, so 10_0 - 10_3 shouldn't have anything else connected to them.

When I connect P10_3 to GND, the reading jumps up to 0xFFFF. When I connect it to VCC 3.6v, the reading goes to about 0x7FF7.

I'm a little bit puzzled as to what's going on here.

-Nick

@kjbracey
Copy link
Contributor

Afraid I'm not familiar with this platform - I just offered the above by inspection. Hopefully @ARMmbed/team-cypress can help.

@nick-atmosphereiot
Copy link
Contributor Author

I think I'm getting somewhere here. The issue is that the Cy_SAR_GetResult16 returns a signed number. When you're at GND, it will hover around that 0xFFFF 0x0000 area and go back and forth. What I can do is check the top bit and set the result to 0 if that bit is set.

So the effective range of the ADC is actually 11 bits plus the one sign bit. That's why VCC is reading at around .5 (0x7FFF).

What I will do is shift the result left 1 bit and then perform the scaling algorithm to 16 bits. Should be good enough, I think, unless anybody has a better idea.

@kyle-cypress
Copy link

Can you please test whether your issue is still reproducible with the latest master? We recently merged in an updated psoc6csp (#11324) which provides some fixes to the ADC HAL, including:

  • Fixing an incorrect value of CYHAL_ADC_MAX_VALUE; it should be 0xFFFF because the ADC hal driver returns a 16-bit value (the physical ADC hardware returns a 12-bit value but that is an implementation detail from the perspective of Cypress HAL clients)
  • Change the SAR vminus connection from VSSA_KELVIN to VREF. This provides increased range (0 - VREF * 2 vs. 0 - VREF) and avoids underflow issues around GND (which are probably the cause of the oscillation between ~0xFFFF and ~0x0000).

@nick-atmosphereiot
Copy link
Contributor Author

nick-atmosphereiot commented Aug 30, 2019 via email

@nick-atmosphereiot
Copy link
Contributor Author

Kyle,

Everything seems to work OK with the latest. The scaling from 12 to 16 bit is still a little bit off, since the maximum 16 bit value your driver will provide is 0xFFF0 instead 0xFFFF, but I think it's good enough.

Thanks,
-Nick

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants