Skip to content

new I2C driver sccb-ng.c is mistakenly byte-swapping reg in SCCB_Write16() and SCCB_Write_Addr16_Val16() #689

Closed
@dhalbert

Description

@dhalbert

Checklist

  • Checked the issue tracker for similar issues to ensure this is not a duplicate
  • Read the documentation to confirm the issue is not addressed there and your configuration is set correctly
  • Tested with the latest version to ensure the issue hasn't been fixed

How often does this bug occurs?

always

Expected behavior

SCCB_Write16() and SCCB_Write_Addr16_Val16() should send the reg address in the correct order to the I2C-connected camera. Instead, the address is byte-swapped.

SCCB_Read16() and SCCB_Read_Addr16_Val16() are fine, and the 8-bit routines are also fine.

Actual behavior (suspected bug)

SCCB_Write16() and SCCB_Write_Addr16_Val16() byte-swap the reg address value twice, instead of just once. Here is the code in SCCB_Read16(), which works:

    uint16_t reg_htons = LITTLETOBIG(reg);
    uint8_t *reg_u8 = (uint8_t *)&reg_htons;

    esp_err_t ret = i2c_master_transmit_receive(dev_handle, reg_u8, 2, rx_buffer, 1, TIMEOUT_MS);

Suppose reg is 0x0102. Then reg_htons will be 0x0201. reg_u8[0] will be 0x01, and reg_u8[1] will be 0x02.

But in SCCB_Write16(), the code is:

    uint16_t reg_htons = LITTLETOBIG(reg);

    uint8_t tx_buffer[3];
    tx_buffer[0] = reg_htons >> 8;
    tx_buffer[1] = reg_htons & 0x00ff;
    tx_buffer[2] = data;

    esp_err_t ret = i2c_master_transmit(dev_handle, tx_buffer, 3, TIMEOUT_MS);

As before, suppose reg is 0x0102. Then reg_htons will be 0x0201. But then tx_buffer[0] will be 0x02, because it is set to reg_htons >> 8. And tx_buffer[1] will be 0x01. This is swapped from what it should be.

Error logs or terminal output

No response

Steps to reproduce the behavior

This occurs whenever a 16-bit register is written. I was converting CircuitPython code to use the new driver_esp_i2c, and updated our code to use the new code in esp32-camera. The camera did not work. I did some read/write/read logging to verify the registers were being written correctly, and they were not.

Project release version

latest master as of 2024-09-27

System architecture

Intel/AMD 64-bit (modern PC, older Mac)

Operating system

Linux

Operating system version

Ubuntu 24.04

Shell

Bash

Additional context

#677, the PR for updating to the new I2C driver, was only tested with an OV2640.

(Thank you @playduck, for the PR! We were blocked without it.)

There is this comment in the PR:

I've translated the functions SCCB_Read16, SCCB_Write16, SCCB_Read_Addr16_Val16 and SCCB_Write_Addr16_Val16 as well. However it seems like these are entirely unused [not true].

So these routines were not tested. But there are uses of these routines for various cameras in sensors/. We use the OV5640, and it needs 16-bit register writes.

I think I can come up with a PR.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions