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

MAX6675 Driver #203

Open
jhoydich opened this issue Oct 5, 2020 · 4 comments
Open

MAX6675 Driver #203

jhoydich opened this issue Oct 5, 2020 · 4 comments
Labels
enhancement New feature or request

Comments

@jhoydich
Copy link

jhoydich commented Oct 5, 2020

I was wondering if anyone has worked on a driver for the MAX6675? I have been porting the adafruit version over to tinygo but ran into an issue with datatypes.


import (
	"machine"
	"time"
)

//Device struct
type Device struct {
	SCK machine.Pin
	CS  machine.Pin
	SO  machine.Pin
}

//Configure MAX6675
func (d Device) Configure() {
	d.SCK.Configure(machine.PinConfig{Mode: machine.PinOutput})
	d.CS.Configure(machine.PinConfig{Mode: machine.PinOutput})
	d.SO.Configure(machine.PinConfig{Mode: machine.PinInput})

	d.CS.High()
}
func (d Device) spiRead() uint16 {
	var i int
	var b uint16 = 0
	for i = 7; i >= 0; i-- {
		d.SCK.Low()
		time.Sleep(time.Nanosecond * 100)
		if d.SO.Get() {
			b |= (1 << i)
		}
		d.SCK.High()
		time.Sleep(time.Nanosecond * 100)
	}

	return b
}

//ReadCelsius Function to get temp in celsius
func (d Device) ReadCelsius() uint16 {
	var temp uint16

	d.CS.Low()
	time.Sleep(time.Microsecond * 10)

	temp |= d.spiRead()
	temp <<= 8
	temp |= d.spiRead()

	d.CS.High()

	if temp == 0x4 {
		return 0
	}

	temp >>= 3

	//t := float64(temp) * .25
	
	return temp
}

Anytime I try to convert the uint to a float the value of t seems to be nil.

Here is a link to the datasheet
And the adafruit library

@jhoydich jhoydich changed the title MAX6675 river MAX6675 Driver Oct 5, 2020
@jhoydich
Copy link
Author

jhoydich commented Oct 7, 2020

It seems to be an issue with floats in general. If I declare a float and try to print it with println then the program will compile and upload but won't display to a serial port. When I try to print a float using fmt.Println or Print I have issues compiling. I have also tried converting from a uint16 to a string and then to a float using the strconv library. Converting to a string is no issue. But again an issue during compiling occurs when converting from a string to a float using ParseFloat.

Edit:
The issue seems to be two fold, the print issue as stated above but there also seems to be an with float math. It turns out converting using float64() works correctly. For use of the chip, I need to divide the value returned by the chip by 4 to get the temperature in Celsius. When either the float is divided or multiplied, some issue occurs. If I try to compare it to another value in an if statement, the program will not work.

//This does not work. Temp is of uint16 type
	t := float64(temp) * .25

	if t > 20 {
		println("Above 20")
	} 

If instead you add or subtract the float by a number, an expression can be evaluated by an if statement.

//This does work. Temp is of uint16 type
	t := float64(temp) - 1.0

	if int(t) + 1 == int(temp) {
		println("Equal")
	}

@conejoninja
Copy link
Member

While floats are generally supported, they tend to generate a number of issues and it's better to avoid them when possible. Sensors in tinygo usually works with millis unit (millidegrees instead of degrees) and uint32 to avoid those issues, for example : https://github.com/tinygo-org/drivers/blob/release/bmp280/bmp280.go#L135

Instead of

t := float64(temp) * .25 // returns degrees
t := uint32(temp) * 250 // returns millidegrees

Also, ReadCelsius() uint16, should be ReadTemperature() (uint32, error), so you could exchange one sensor for another without refactoring the code

@jhoydich
Copy link
Author

Awesome! Thank you! I'll make those changes soon and test when I am home in a couple weeks.

I'm guessing I should also change the receiver of ReadTemperature() (as well as Configure() and spiRead()) to be a pointer instead of a value?

I'm also assuming I should really be passing an spi bus to the New() function and using the built in tx() method to read from the sensor?

Sorry for all of the questions, this is the first time I've tried writing a driver. I find this project very interesting overall and I hope I can eventually contribute what (little) I can in the future.

@conejoninja
Copy link
Member

If you are using a spi transport you should be passing it already configured to the device New function.
Existing drivers are a good template for new ones. Don't worry and make as many questions as needed, we're happy to welcome you to our little community and grateful you want to contribute!

@deadprogram deadprogram added the enhancement New feature or request label Oct 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants