OneWireFirmata does not work on esp8266 and solution

[jcvillegasfernandez]

Trying my Lazarus client over wifi I realized that the module OneWireFirmata does not work on esp8266, exception issue (some thing about a bad alignment), but OneWire library worked.

After a lot of researching I think I have discovered the wrong code:

if (subcommand & ONEWIRE_READ_REQUEST_BIT) {
if (numBytes < 4) break;
// this is the bad part
numReadBytes = ((int)argv);
argv += 2;
correlationId = ((int)argv);
argv += 2;
// this is the right one
numReadBytes = argv[0] | ((int)argv[1] << 8);
correlationId = argv[2] | ((int)argv[3] << 8);
argv += 4;

and you have to change these lines too:
if (subcommand & ONEWIRE_DELAY_REQUEST_BIT) {
if (numBytes < 4) break;
// bad code
Firmata.delayTask(((long)argv));
// this is the right one
Firmata.delayTask(argv[0] | ((long)argv[1] << 8) | ((long)argv[2] << 16) | ((long)argv[3] << 24));

[mkormout]

Thank you, it works, great work!!!
What about pull request? :)

[jcvillegasfernandez]

I only try to use configurablefirmata for my Lazarus client and if I find something wrong when I test my client I try to fix it but I am not an arduino programmer.

[lawern]

Thank you, it works for me too!

[echavet]

#136 (comment)

[pgrawehr]

This should be fixed with #136. Please reopen if not.

[echavet]

I know this is a closed issue but...

This is a bit surprising, given that all the platforms (esp8266, ardiono uno or zero for me) in question are little endian and the "long" data type is consistently 32 bits in size. Despite this, users have reported that the proposed code change resolves the issue!

For me, this problem arises when working with Arduino Uno, but not with Arduino Zero. Interestingly, the occurrence of the error seems to be contingent upon the inclusion of FirmataScheduler, as defined by the configuration typedef constant:

#ifdef ENABLE_BASIC_SCHEDULER

Looking for an explanation, I discovered that the implementation of the delayTaskCallback function is specific to FirmataScheduler. It calls Firmata.attachDelayTask(delayTaskCallback); for its software delay implementation:

void FirmataScheduler::delayTask(long delay_ms)
{
  if (running) {
    long now = millis();
    running->time_ms += delay_ms;
    if (running->time_ms < now) { // If delay time already passed, schedule to 'now'.
      running->time_ms = now;
    }
  }
}

I am no longer able to test this, but I believe that without ENABLE_BASIC_SCHEDULER, there is NO implementation for delayTaskCallback. Therefore, the delay operation is solely dependent on the runtime execution of the delayTask method:

if (delayTaskCallback) {
  (*delayTaskCallback)(delay);
}

Now let's consider the comparison between these two scenarios:

1. Three left shifts and three long casts: argv[0] | ((long)argv[1] << 8) | ((long)argv[2] << 16) | ((long)argv[3] << 24)
2. Versus a single long cast: Firmata.delayTask(*((long*)argv));

Given that I'm using the Johnny-Five library, and its implementation of the DS18B20 thermometer uses board.io.sendOneWireDelay(pin, 1); to allow for temperature conversion, it's plausible that this delay is primarily necessary for slower platforms.

My hypothesis is that there would be no discernible difference with a longer delay period when delayTaskCallback is null because there is no delay instruction.

/**
 * Call the delayTask callback function when using FirmataScheduler. Must first attach a callback
 * using attachDelayTask.
 * @see FirmataScheduler
 * @param delay The amount of time to delay in milliseconds.
 */
void FirmataClass::delayTask(long delayMs) {
  if (delayTaskCallback) {
    (*delayTaskCallback)(delayMs);
  } 
  // adding a default implementation seems to solve the issue
   else {
    delay(delayMs);
  }
}

---