hubsif / arduino-dali

A timer-based DALI (Digital Addressable Lighting Interface) library for Arduino
https://hubsif.github.io/arduino-dali/
GNU General Public License v3.0
26 stars 19 forks source link

COMMISSION_CHECKFOUND does not handle error from DaliBus.getLastResponse() #3

Closed milo1000 closed 11 months ago

milo1000 commented 1 year ago
      case COMMISSION_CHECKFOUND:
        {  // create scope for response variable
        int response = DaliBus.getLastResponse();
        if (response != DALI_RX_EMPTY) {
          if (searchIterations >= 24) // ballast found
            commissionState = COMMISSION_PROGRAMSHORT;
          else {
            currentSearchAddress -= (0x800000 >> searchIterations);
            commissionState = COMMISSION_SEARCHHIGH;
          }
        }

In above code in Dali.cpp DaliBus.getLastResponse() can return DALI_RX_ERROR and it's not handled properly (I am not sure how it should be handled, but now it's definitely wrong). This is actual scenario - happened to me when RX_START didn't meet criteria from isDeltaWithinTE(delta) (delta was bit smaller than expected 333). I don't know why that's the case, but it's another story (BTW. spec claims that those collision rules shall not apply to responses, but I am not competent here).

hubsif commented 1 year ago

Hi @milo1000! Thank you for your feedback. You're right, DALI_RX_ERROR is not handled properly here. I'll try to fix that with the next update.

(BTW. spec claims that those collision rules shall not apply to responses, but I am not competent here)

I'm not sure what you're referring to here. From what I understand this is not about collision detection. If _RXSTART didn't meet criteria from isDeltaWithinTE(delta), your ballast pulled down the bus and released it too soon, which doesn't comply to DALI's Manchester protocol specifications and thus is an error. (Ok, I just checked: Actually the official minimum limit is not 333µs but 325µs, i.e. 416.67µs half-bit time - 10% tolerance - 50µs half-slope time), but that seems insignificant 😄)

milo1000 commented 1 year ago

From what I understand this is not about collision detection

Yeah, you probably right, as I said, I am not an expert in manchester. As you said it's probably something with my ballast, probably i need an oscilloscope to check this. BTW. Thanks for the great job, without your "commision" procedure I would never figure out how that stuff works.

hubsif commented 11 months ago

Seems to have been solved here:

For me it helped to increase DALI_TE_MAX limit, like: const unsigned long DALI_TE_MAX = (130 * DALI_TE) / 100; I am not sure it's the same problem though.