RobTillaart / MCP4725

Arduino library for 12 bit I2C DAC - MCP4725
MIT License
24 stars 7 forks source link

setValue hangs (freeze) if device with address is not connected #6

Closed svatos-jirka closed 3 years ago

svatos-jirka commented 3 years ago

Hi,

setValue hangs (freeze) if device with address is not connected

RobTillaart commented 3 years ago

That sounds like a serious problem. Think I did not included that scenario in my tests. What platform do you use? UNO ESP? So I can try to recreate it.

Might take a few days due to the holidays

svatos-jirka commented 3 years ago

Hi,

I have node mcu (esp8266),

my observation: The issue starts in begin() and the device is not connected. I think, that begin() tries to read value of DAC and saves it to _lastValue. Is there way how to skip this step of reading? Must I use .begin() and then .setValue() or could I use .setValue() only?

with best regards, jiri

RobTillaart commented 3 years ago

Q: if the sensor is connected, does the library work?

RobTillaart commented 3 years ago

The issue starts in begin() and the device is not connected. I think, that begin() tries to read value of DAC and saves it to _lastValue. Is there way how to skip this step of reading? Must I use .begin() and then .setValue() or could I use .setValue() only?

Hi Jiri,

I have no hardware nearby but your analysis could be right.

First step I need to do is add continuous integration to the library so I have some minimal automated test. Then I will add a isConnected() function that should verify is a sensor is available. That should be inside begin() preventing the blocking.

If that works this check should be all over the place where it tries to connect. To be continued.

svatos-jirka commented 3 years ago

Hi,

this example works, if device is connected (0x61)

#include <Wire.h>
#include <ESP8266WiFi.h>
#include <PubSubClient.h>
#include "PCF8591.h"
#include "MCP4725.h"
#define OUTPUTS_N 2
#define INPUTS_N 1

const char* ssid = "--------------";
const char* password = "-----------";
const char* mqtt_server = "------------";

WiFiClient espClient;
PubSubClient client(espClient);
const char* lightsTopicName = "lights/hall/";
int lightTopicLenght = 12;
char msg[50];

// I/O units
struct InputUnit {
  int id;     // used for mqtt
  int multiplex;  // address of multiplexer
  int bus;        // bus 0-7
  int port;       // 0 - 4, typical 2
  PCF8591 driver;  
  int value;
};

struct OutputUnit {
  int id;     // used for mqtt
  int multiplex;  // address of multiplexer
  int bus;        // bus 0-7
  int type;       // 1 PCF8591, 2 MCP4725
  PCF8591 pcf;  
  MCP4725 mcp;
  int value;
  int target;
};

struct InputUnit hall_1_1 = {1, 0x70, 0,  2,  PCF8591(0x48),0};
struct InputUnit inputs[INPUTS_N] = {hall_1_1};

struct OutputUnit hall_3 = {1,0x70,0,1, PCF8591(0x48),(MCP4725)NULL ,0,255};
struct OutputUnit hall_2 = {2,0x70,1,2,(PCF8591)NULL,  MCP4725(0x61),0,255};

struct OutputUnit outputs[OUTPUTS_N] = {hall_2,hall_3};

void setup_wifi() {
  delay(10);
  Serial.println();
  Serial.print("Connecting to ");
  Serial.println(ssid);
  WiFi.begin(ssid, password);
  while (WiFi.status() != WL_CONNECTED) {
    delay(500);
    Serial.print(".");
  }

  randomSeed(micros());
  Serial.println("IP address: ");
  Serial.println(WiFi.localIP());
}

void callback(char* topic, byte* payload, unsigned int length) {
  Serial.print("Message arrived [");
  Serial.print(topic);
  Serial.print("] ");
  for (int i = 0; i < length; i++) {
    Serial.print((char)payload[i]);
  }
  Serial.println();
  int id = String(topic).substring(lightTopicLenght).toInt();
  Serial.print("Light  #");
  Serial.print(id);
  Serial.print(" target:");

  for(int i =0; i<OUTPUTS_N;i++){
    if(outputs[i].id == id){
       outputs[i].target=String((char*)payload).substring(0,length).toInt();
       Serial.println(outputs[i].target);
      break;
    }
  }
}

void reconnect() {
  // Loop until we're reconnected
  while (!client.connected()) {
    Serial.print("Attempting MQTT connection...");
    // Create a random client ID
    String clientId = "ESP8266Client-";
    clientId += String(random(0xffff), HEX);
    // Attempt to connect
    if (client.connect(clientId.c_str())) {
       for(int i=0;i<OUTPUTS_N;i++){
        client.subscribe((char*) String(String(lightsTopicName)+String(outputs[i].id)).c_str());
        Serial.print("Subscribe to: \t");
        Serial.println(outputs[i].id);
      }
    } else {
      Serial.print("failed, rc=");
      Serial.print(client.state());
      Serial.println(" try again in 5 seconds");
      // Wait 5 seconds before retrying
      delay(5000);
    }
  }
}

void busSelect(int addr,int i) {
  if (i > 7) return;
  Wire.beginTransmission(addr);
  Wire.write(1 << i);
  Wire.endTransmission();  
}

int step(int diff){
  if(diff == 0) return 0;
  if(diff  <0) return -1;
  if(diff  >0) return 1;
}

void setup() {
  Serial.begin(115200);
  setup_wifi();
  client.setServer(mqtt_server, 1883);
  client.setCallback(callback);
}

void loop() {
 if (!client.connected()) {
  reconnect();
  }
  client.loop();

  for(int i=0;i<INPUTS_N;i++){
      InputUnit input = inputs[i];
      busSelect(input.multiplex,input.bus);
      input.driver.begin();
      int val = input.driver.analogRead(2);
      snprintf (msg, 10, "pirs/%ld", input.id);
      if(inputs[i].value != val){
        inputs[i].value= val;
        client.publish((char*)  String("pirs/"+String(input.id)).c_str(), (char*) String(val).c_str());    
        Serial.print("Publish message: ");
        Serial.print(msg);
        Serial.print(":");
        Serial.println(inputs[i].value);
  }
  }

  for(int i=0;i<2;i++){
    OutputUnit output = outputs[i];
    busSelect(output.multiplex,output.bus);   
    int diff = step(output.target-output.value);
    outputs[i].value = outputs[i].value+diff ;

    if(diff != 0 ){

      switch(output.type){
        case 1:
          Serial.println("PCF");
          output.pcf.begin();
          output.pcf.analogWrite(output.value);
        break;
        case 2:
          Serial.println("MCP");
          output.mcp.begin();
          output.mcp.setValue(output.value*16-1);
        break;
      }
    }
  }
  delay(1);
}

IF device is not connected, then it hangs with text "MCP" on console.

Thanks for advice and hints how to improve it (maybe the begin function should be on different location)

RobTillaart commented 3 years ago

Updated your post add quotes+cpp around the code to let it look better.

RobTillaart commented 3 years ago

You can give it a quick try by commenting line 50 // _lastValue = readDAC(); and the line 51 // _powerDownMode = readPowerDownModeDAC();

both reads can block.

To be continued tomorrow as Christmas "calls"

svatos-jirka commented 3 years ago

Thanks a lot.

I changed it in file C:...\Documents\Arduino\libraries\MCP4725\MCP47255.cpp I saw in debug output: Using library MCP4725 at version 0.2.2 in folder: C:....\Documents\Arduino\libraries\MCP4725

void MCP4725::begin(const uint8_t dataPin, const uint8_t clockPin)
{
  Wire.begin(dataPin, clockPin);
  Serial.println("Begin");
  // Wire.setClock(100000UL);
  //_lastValue = readDAC();
  ///_powerDownMode = readPowerDownModeDAC();
}

but I dont have any good news.

RobTillaart commented 3 years ago

automatic build works now, that allows some testing.

RobTillaart commented 3 years ago

The branch - https://github.com/RobTillaart/MCP4725/tree/isConnected now has an example to test isConnected() it is a minimal example but that's the first one that should work.

RobTillaart commented 3 years ago

@svatos-jirka The isConnected() function is now merged. You can use it before any function call to see if the MCP is available. There are still problems possible if the MCP is removed just after isConnected() returned true and before you called e.g. readDAC(). Chances are slim that that happens, but that need a bit more thinking to be solved more robustly. For now you should be able to have less blocking calls as you can check the availability. .

I also do a connection check in begin() which returns true if the sensor is connected. Then it will readback some values from the MCP, otherwise it will set those values to 0.

Can you verify if this is working for you? (you might need to add some calls to your sketch of course) Thanks,

svatos-jirka commented 3 years ago

It works! Thanks you very much!

RobTillaart commented 3 years ago

OK, then I will close the issue