- fixed bug in Pin::attachInterrupt (this is an arduino bug : interrupts that have been triggered before the handler is attached will cause it to be called immediately ; but only interrupts happening after should call the handler)

- changed OneWireSlave interface to make it completely static (there is an instance, but it's only a wrapper to the static stuff, so that the syntax looks nice)
- the search rom algorithm works fine most of the time, but sometimes it fails, probably because of interrupts being blocked at the wrong time (small delay to respond to the master to send a zero)

LowLevel.h

@@ -76,6 +76,12 @@ private:
 	byte interruptNumber_;
 
 public:
+	Pin()
+		: mask_(0)
+		, reg_(0)
+		, interruptNumber_((byte)-1)
+	{ }
+
 	Pin(uint8_t pin)
 	{
 		mask_ = PIN_TO_BITMASK(pin);
@@ -97,7 +103,11 @@ public:
 	inline void writeHigh() { DIRECT_WRITE_HIGH(reg_, mask_); }
 	inline void write(bool value) { if (value) writeHigh(); else writeLow(); }
 
-	inline void attachInterrupt(void (*handler)(), int mode) { ::attachInterrupt(interruptNumber_, handler, mode); }
+	inline void attachInterrupt(void (*handler)(), int mode)
+	{
+		EIFR |= (1 << INTF0); // clear any pending interrupt (we want to call the handler only for interrupts happending after it is attached)
+		::attachInterrupt(interruptNumber_, handler, mode);
+	}
 	inline void detachInterrupt() { ::detachInterrupt(interruptNumber_); }
 };
 

OneWireIO.ino

@@ -11,14 +11,13 @@ SerialChannel debug("debug");
 Pin led(LEDPin);
 
 byte owROM[7] = { 0xE2, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02 };
-OneWireSlave oneWire(owROM, OWPin);
 
 void setup()
 {
 	led.outputMode();
 	led.writeLow();
 
-	oneWire.enable();
+	OneWire.begin(owROM, OWPin);
     
     Serial.begin(9600);
 }
@@ -40,7 +39,7 @@ void loop()
 	SerialChannel::flush();
 
 	byte b;
-	if (oneWire.read(b))
+	if (OneWire.read(b))
 	{
 		char msg[32];
 		sprintf(msg, "Received byte : %d", (int)b);

OneWireSlave.cpp

@@ -25,7 +25,25 @@ namespace
 	void(*timerEvent)() = 0;
 }
 
-OneWireSlave* OneWireSlave::inst_ = 0;
+OneWireSlave OneWire;
+
+byte OneWireSlave::rom_[8];
+Pin OneWireSlave::pin_;
+byte OneWireSlave::tccr1bEnable_;
+
+unsigned long OneWireSlave::resetStart_;
+unsigned long OneWireSlave::lastReset_;
+
+void(*OneWireSlave::receiveBitCallback_)(bool bit, bool error);
+void(*OneWireSlave::bitSentCallback_)(bool error);
+
+byte OneWireSlave::receivingByte_;
+byte OneWireSlave::receivingBitPos_;
+byte OneWireSlave::receiveTarget_;
+
+byte OneWireSlave::searchRomBytePos_;
+byte OneWireSlave::searchRomBitPos_;
+bool OneWireSlave::searchRomInverse_;
 
 ISR(TIMER1_COMPA_vect) // timer1 interrupt
 {
@@ -35,38 +53,37 @@ ISR(TIMER1_COMPA_vect) // timer1 interrupt
 	event();
 }
 
-OneWireSlave::OneWireSlave(byte* rom, byte pinNumber)
-	: pin_(pinNumber)
-	, resetStart_((unsigned long)-1)
-	, lastReset_(0)
-	, ignoreNextEdge_(false)
+void OneWireSlave::begin(byte* rom, byte pinNumber)
 {
-	inst_ = this; // we can have only one instance in the current implementation
+	pin_ = Pin(pinNumber);
+	resetStart_ = (unsigned long)-1;
+	lastReset_ = 0;
+
 	memcpy(rom_, rom, 7);
 	rom_[7] = crc8_(rom_, 7);
-}
 
-void OneWireSlave::enable()
-{
 	#ifdef DEBUG_LOG
 	debug.append("Enabling 1-wire library");
 	dbgOutput.outputMode();
 	dbgOutput.writeHigh();
 	#endif
 
-
 	cli(); // disable interrupts
 	pin_.inputMode();
+	pin_.writeLow(); // make sure the internal pull-up resistor is disabled
+
 	// prepare hardware timer
 	TCCR1A = 0;
 	TCCR1B = 0;
 	TIMSK1 |= (1 << OCIE1A); // enable timer compare interrupt
 	tccr1bEnable_ = (1 << WGM12) | (1 << CS11) | (1 << CS10); // turn on CTC mode with 64 prescaler
+
+	// start 1-wire activity
 	beginWaitReset_();
 	sei(); // enable interrupts
 }
 
-void OneWireSlave::disable()
+void OneWireSlave::end()
 {
 	#ifdef DEBUG_LOG
 	debug.append("Disabling 1-wire library");
@@ -165,7 +182,7 @@ void OneWireSlave::releaseBus_()
 void OneWireSlave::beginWaitReset_()
 {
 	disableTimer_();
-	pin_.attachInterrupt(&OneWireSlave::waitResetHandler_, CHANGE);
+	pin_.attachInterrupt(&OneWireSlave::waitReset_, CHANGE);
 	resetStart_ = (unsigned int)-1;
 }
 
@@ -195,7 +212,7 @@ void OneWireSlave::waitReset_()
 
 			lastReset_ = now;
 			pin_.detachInterrupt();
-			setTimerEvent_(PresenceWaitDuration - (micros() - now), &OneWireSlave::beginPresenceHandler_);
+			setTimerEvent_(PresenceWaitDuration - (micros() - now), &OneWireSlave::beginPresence_);
 		}
 	}
 	else
@@ -209,7 +226,7 @@ void OneWireSlave::beginPresence_()
 {
 	unsigned long now = micros();
 	pullLow_();
-	setTimerEvent_(PresenceDuration, &OneWireSlave::endPresenceHandler_);
+	setTimerEvent_(PresenceDuration, &OneWireSlave::endPresence_);
 	#ifdef DEBUG_LOG
 	debug.SC_APPEND_STR_TIME("reset", lastReset_);
 	debug.SC_APPEND_STR_TIME("beginPresence", now);
@@ -224,7 +241,6 @@ void OneWireSlave::endPresence_()
 	debug.SC_APPEND_STR_TIME("endPresence", now);
 	#endif
 
-	ignoreNextEdge_ = true;
 	beginWaitCommand_();
 }
 
@@ -238,26 +254,21 @@ void OneWireSlave::beginReceive_()
 {
 	receivingByte_ = 0;
 	receivingBitPos_ = 0;
-	beginReceiveBit_(&OneWireSlave::onBitReceivedHandler_);
+	beginReceiveBit_(&OneWireSlave::onBitReceived_);
 }
 
 void OneWireSlave::beginReceiveBit_(void(*completeCallback)(bool bit, bool error))
 {
 	receiveBitCallback_ = completeCallback;
-	pin_.attachInterrupt(&OneWireSlave::receiveHandler_, FALLING);
+	pin_.attachInterrupt(&OneWireSlave::receive_, FALLING);
 }
 
 void OneWireSlave::receive_()
 {
 	onEnterInterrupt_();
 
-	if (!ignoreNextEdge_)
-	{
-		pin_.detachInterrupt();
-		setTimerEvent_(ReadBitSamplingTime, &OneWireSlave::readBitHandler_);
-	}
-
-	ignoreNextEdge_ = false;
+	pin_.detachInterrupt();
+	setTimerEvent_(ReadBitSamplingTime, &OneWireSlave::readBit_);
 
 	onLeaveInterrupt_();
 }
@@ -267,11 +278,11 @@ void OneWireSlave::beginSendBit_(bool bit, void(*completeCallback)(bool error))
 	bitSentCallback_ = completeCallback;
 	if (bit)
 	{
-		pin_.attachInterrupt(&OneWireSlave::sendBitOneHandler_, FALLING);
+		pin_.attachInterrupt(&OneWireSlave::sendBitOne_, FALLING);
 	}
 	else
 	{
-		pin_.attachInterrupt(&OneWireSlave::sendBitZeroHandler_, FALLING);
+		pin_.attachInterrupt(&OneWireSlave::sendBitZero_, FALLING);
 	}
 }
 
@@ -279,28 +290,19 @@ void OneWireSlave::sendBitOne_()
 {
 	onEnterInterrupt_();
 
-	bool ignoreEdge = ignoreNextEdge_;
-	ignoreNextEdge_ = false;
-
-	if (!ignoreEdge)
-		bitSentCallback_(false);
+	bitSentCallback_(false);
 
 	onLeaveInterrupt_();
 }
 
 void OneWireSlave::sendBitZero_()
 {
-	onEnterInterrupt_();
+	pullLow_(); // this must be executed first because the timing is very tight with some master devices
 
-	if (ignoreNextEdge_)
-	{
-		ignoreNextEdge_ = false;
-		return;
-	}
+	onEnterInterrupt_();
 
-	pullLow_();
 	pin_.detachInterrupt();
-	setTimerEvent_(SendBitDuration, &OneWireSlave::endSendBitZeroHandler_);
+	setTimerEvent_(SendBitDuration, &OneWireSlave::endSendBitZero_);
 
 	onLeaveInterrupt_();
 }
@@ -366,7 +368,7 @@ void OneWireSlave::onBitReceived_(bool bit, bool error)
 		}
 	}
 	
-	beginReceiveBit_(&OneWireSlave::onBitReceivedHandler_);
+	beginReceiveBit_(&OneWireSlave::onBitReceived_);
 }
 
 void OneWireSlave::beginSearchRom_()
@@ -384,7 +386,7 @@ void OneWireSlave::beginSearchRomSendBit_()
 	bool currentBit = bitRead(currentByte, searchRomBitPos_);
 	bool bitToSend = searchRomInverse_ ? !currentBit : currentBit;
 
-	beginSendBit_(bitToSend, &OneWireSlave::continueSearchRomHandler_);
+	beginSendBit_(bitToSend, &OneWireSlave::continueSearchRom_);
 }
 
 void OneWireSlave::continueSearchRom_(bool error)
@@ -403,7 +405,7 @@ void OneWireSlave::continueSearchRom_(bool error)
 	}
 	else
 	{
-		beginReceiveBit_(&OneWireSlave::searchRomOnBitReceivedHandler_);
+		beginReceiveBit_(&OneWireSlave::searchRomOnBitReceived_);
 	}
 }
 

OneWireSlave.h

@@ -7,14 +7,11 @@
 class OneWireSlave
 {
 public:
-	///! Constructs a 1-wire slave that will be identified by the specified ROM (7 bytes, starting from the family code, CRC will be computed internally). Call enable to actually start listening for the 1-wire master.
-	OneWireSlave(byte* rom, byte pinNumber);
-
-	///! Starts listening for the 1-wire master. Reset, Presence and SearchRom are handled automatically. The library will use interrupts on the pin specified in the constructor, as well as one hardware timer. Blocking interrupts (either by disabling them explicitely with sei/cli, or by spending time in another interrupt) can lead to malfunction of the library, due to tight timing for some 1-wire operations.
-	void enable();
+	///! Starts listening for the 1-wire master, on the specified pin, as a virtual slave device identified by the specified ROM (7 bytes, starting from the family code, CRC will be computed internally). Reset, Presence, SearchRom and MatchRom are handled automatically. The library will use the external interrupt on the specified pin (note that this is usually not possible with all pins, depending on the board), as well as one hardware timer. Blocking interrupts (either by disabling them explicitely with sei/cli, or by spending time in another interrupt) can lead to malfunction of the library, due to tight timing for some 1-wire operations.
+	void begin(byte* rom, byte pinNumber);
 
 	///! Stops all 1-wire activities, which frees hardware resources for other purposes.
-	void disable();
+	void end();
 
 	///! Pops one byte from the receive buffer, or returns false if no byte has been received.
 	bool read(byte& b);
@@ -26,74 +23,62 @@ public:
 	void write(byte* bytes, short numBytes, void(*complete)(bool error));
 
 private:
-	byte crc8_(byte* data, short numBytes);
+	static byte crc8_(byte* data, short numBytes);
 
-	void setTimerEvent_(short delayMicroSeconds, void(*handler)());
-	void disableTimer_();
+	static void setTimerEvent_(short delayMicroSeconds, void(*handler)());
+	static void disableTimer_();
 
-	void onEnterInterrupt_();
-	void onLeaveInterrupt_();
+	static void onEnterInterrupt_();
+	static void onLeaveInterrupt_();
 
-	void error_(const char* message);
+	static void error_(const char* message);
 
-	void pullLow_();
-	void releaseBus_();
+	static void pullLow_();
+	static void releaseBus_();
 
-	void beginWaitReset_();
-	void beginWaitCommand_();
-	void beginReceive_();
+	static void beginReceiveBit_(void(*completeCallback)(bool bit, bool error));
+	static void beginSendBit_(bool bit, void(*completeCallback)(bool error));
 
-	void beginReceiveBit_(void(*completeCallback)(bool bit, bool error));
-	void beginSendBit_(bool bit, void(*completeCallback)(bool error));
+	static void beginWaitReset_();
+	static void beginWaitCommand_();
+	static void beginReceive_();
+	static void onBitReceived_(bool bit, bool error);
 
-	void beginSearchRom_();
-	void beginSearchRomSendBit_();
-	inline static void continueSearchRomHandler_(bool error) { inst_->continueSearchRom_(error); }
-	void continueSearchRom_(bool error);
-	inline static void searchRomOnBitReceivedHandler_(bool bit, bool error) { inst_->searchRomOnBitReceived_(bit, error); }
-	void searchRomOnBitReceived_(bool bit, bool error);
+	static void beginSearchRom_();
+	static void beginSearchRomSendBit_();
+	static void continueSearchRom_(bool error);
+	static void searchRomOnBitReceived_(bool bit, bool error);
 
 	// interrupt handlers
-	inline static void waitResetHandler_() { inst_->waitReset_(); }
-	void waitReset_();
-	inline static void beginPresenceHandler_() { inst_->beginPresence_(); }
-	void beginPresence_();
-	inline static void endPresenceHandler_() { inst_->endPresence_(); }
-	void endPresence_();
-	inline static void receiveHandler_() { inst_->receive_(); }
-	void receive_();
-	inline static void readBitHandler_() { inst_->readBit_(); }
-	void readBit_();
-	inline static void onBitReceivedHandler_(bool bit, bool error) { inst_->onBitReceived_(bit, error); }
-	void onBitReceived_(bool bit, bool error);
-	inline static void sendBitOneHandler_() { inst_->sendBitOne_(); }
-	void sendBitOne_();
-	inline static void sendBitZeroHandler_() { inst_->sendBitZero_(); }
-	void sendBitZero_();
-	inline static void endSendBitZeroHandler_() { inst_->endSendBitZero_(); }
-	void endSendBitZero_();
+	static void waitReset_();
+	static void beginPresence_();
+	static void endPresence_();
+	static void receive_();
+	static void readBit_();
+	static void sendBitOne_();
+	static void sendBitZero_();
+	static void endSendBitZero_();
 
 private:
-	static OneWireSlave* inst_;
-	byte rom_[8];
-	Pin pin_;
-	byte tccr1bEnable_;
-
-	unsigned long resetStart_;
-	unsigned long lastReset_;
+	static byte rom_[8];
+	static Pin pin_;
+	static byte tccr1bEnable_;
 
-	void(*receiveBitCallback_)(bool bit, bool error);
-	void(*bitSentCallback_)(bool error);
+	static unsigned long resetStart_;
+	static unsigned long lastReset_;
 
-	byte receivingByte_;
-	byte receivingBitPos_;
-	byte receiveTarget_;
+	static void(*receiveBitCallback_)(bool bit, bool error);
+	static void(*bitSentCallback_)(bool error);
 
-	bool ignoreNextEdge_;
+	static byte receivingByte_;
+	static byte receivingBitPos_;
+	static byte receiveTarget_;
 
-	byte searchRomBytePos_;
-	byte searchRomBitPos_;
-	bool searchRomInverse_;
+	static byte searchRomBytePos_;
+	static byte searchRomBitPos_;
+	static bool searchRomInverse_;
 };
 
+extern OneWireSlave OneWire;
+
 #endif