feature-requests icon indicating copy to clipboard operation
feature-requests copied to clipboard

Support ds2482-800

Open alainstark opened this issue 10 months ago • 26 comments

Hello, I have write code to support ds2482-800 to the component ds248x

I have comment this feature at the end of this feature-request

I am not a git expert, can you help me to add the ds248x component to esphome ?

alainstark avatar Apr 18 '24 10:04 alainstark

Post a link

nagyrobi avatar Apr 18 '24 18:04 nagyrobi

ds248x.zip

This is the code. Il that OK ?

alainstark avatar Apr 18 '24 20:04 alainstark

Excuse me but I'm really bad at git. I closed this request by mistake. I don't know how to go back. Should I open a new request?

alainstark avatar Apr 19 '24 08:04 alainstark

@alainstark We need a doc description too, could you make it?

nagyrobi avatar Apr 19 '24 09:04 nagyrobi

Yes, is this link the right thing to learn ?

alainstark avatar Apr 19 '24 10:04 alainstark

ds248x.zip

This is the code. Il that OK ?

First of all: thank you!!!

I tried this out, it works me only with "index", not working (no temperature data) if i use the sensor "address" (without index) - my settings:

external_components:

  • source:
    type: local path: components components:
    • ds248x

ds248x: type: ds2482-800 i2c_id: i2c_bus_1 address: 0x18 id: dallas_1 #sleep_pin: 32 active_pullup: true #strong_pullup: false #hub_sleep: false #bus_sleep: false update_interval: 3s

sensor:

  • platform: ds248x address: 0x0d00000ac7f07228 name: ds2482-800-ch0-1 id: ds2482_800_ch0_1 channel: 0 #index: 0 accuracy_decimals: 2
  • platform: ds248x address: 0x0d010851cb61af38 name: ds2482-800-ch1-1 id: ds2482_800_ch1_1 channel: 1 #index: 1 accuracy_decimals: 2

What is wrong with it? Can you help me? Do you have a newer version of the code?

csabter avatar Apr 21 '24 12:04 csabter

Have you incrase the verbosity of the logs ?

alainstark avatar Apr 21 '24 16:04 alainstark

I belive that you should not use id: ds2482_800_ch1_1. This syntax is for multiple ds248x on the same I²C bus. Comment this line and test again.

alainstark avatar Apr 21 '24 17:04 alainstark

And this is the doc :-) ds248x-doc.zip

alainstark avatar Apr 21 '24 17:04 alainstark

Thank you very much!

It's work! :)

I don't know why, but there were some indentation problem in the sensor.py file (i unziped and opened it from Linux, Geany and Kate):

After unzip: [...] async def to_code(config): hub = await cg.get_variable(config[CONF_DALLAS_ID]) var = await sensor.new_sensor(config)

if CONF_ADDRESS in config:
    cg.add(var.set_address(config[CONF_ADDRESS]))
else:
    cg.add(var.set_index(config[CONF_INDEX]))
    
    if CONF_RESOLUTION in config:
        cg.add(var.set_resolution(config[CONF_RESOLUTION]))
        
        if CONF_CHANNEL in config:
            cg.add(var.set_channel(config[CONF_CHANNEL]))
            
            cg.add(var.set_parent(hub))
            
            cg.add(hub.register_sensor(var))

After orrection: [...] async def to_code(config): hub = await cg.get_variable(config[CONF_DALLAS_ID]) var = await sensor.new_sensor(config)

if CONF_ADDRESS in config:
    cg.add(var.set_address(config[CONF_ADDRESS]))
else:
    cg.add(var.set_index(config[CONF_INDEX]))
    
if CONF_RESOLUTION in config:
    cg.add(var.set_resolution(config[CONF_RESOLUTION]))
    
if CONF_CHANNEL in config:
    cg.add(var.set_channel(config[CONF_CHANNEL]))
        
cg.add(var.set_parent(hub))
        
cg.add(hub.register_sensor(var))

I will change the id sytax.

I am very gateful for your help and this code and documentation!

(Sorry, my English is not very good...)

csabter avatar Apr 21 '24 19:04 csabter

Yes it's my fault. Before generating the zip archive, I opened the code in emacs and mechanically indented the code without testing. I'm going to try to train myself in git to set up this new component in esphome. If anyone wants to help me, I'm all ears. Concerning English, I myself (I am French) am not an expert and I admit to using Google translate.

alainstark avatar Apr 22 '24 10:04 alainstark

Less pro, but can be done even from web UI of GitHub. Clone ESPHome repo to your account. Create a new branch from DEV. Create a new subdirectory in the components for your component. Upload the files. Contribute back by comparing changes against dev branches, and making a pull request. Similar for doc.

nagyrobi avatar Apr 22 '24 11:04 nagyrobi

I'd like to help, but unfortunately i don't know git at all (basically, i work with hardware), i hope you can somehow get it together with ESPHome Repo! (I'll write as soon as i find someone who could help.) I think it absolutely fills a gap! Thank you!

csabter avatar Apr 22 '24 14:04 csabter

Ok nagyrobi. I used the non pro method. Is that correct ? esphome esphome_doc If yes, what must I do yet ?

alainstark avatar Apr 22 '24 17:04 alainstark

Now you see in your esphome repo, at the top of the page a link "This branch is 5 commits ahead of". Click on "5 commits ahead of" and make the pull request. Make sure you complete the exact instructions.

For the docs similar, but please make sure that in the Comparing changes window you correct manually the "Base repository" base to be "next" not "current" !!

Whet you're done, you can look at your PR and edit the first post to add the required PR links to connect the two to each-other.

nagyrobi avatar Apr 23 '24 07:04 nagyrobi

Great! Added some comments on the next steps in the PRs.

nagyrobi avatar Apr 23 '24 13:04 nagyrobi

Okay, I think I'm almost there.

I still have a problem with updating CODEOWNERS. I am not the principal writer of this component. I have only adapt this to support ds2482-800 the principal writer is mkncj. mkncj/esphome

I wait for the approwing review.

alainstark avatar Apr 25 '24 08:04 alainstark

You can safely put yourself as codeowner because the goal is to have the system notify somebody if the component needs some changes in the future, that's all.

nagyrobi avatar Apr 25 '24 11:04 nagyrobi

Ok I have do it.

alainstark avatar Apr 25 '24 14:04 alainstark

I think I didn't do the right thing regarding CODEOWNERS. In the PR, the needs-codeowners label is always present. I have changed init.py and run script/build_codeowners.py The script did not give me any errors. I then wanted to commit the modification (in init.py) which restarted the verification tool which went into error. So, I went back. What should I do ?

alainstark avatar Apr 26 '24 18:04 alainstark

Maybe the script is broken. Let your codeowners in, I removed the label manually.

nagyrobi avatar Apr 26 '24 20:04 nagyrobi

Ok I added the line again that give me an error in script/ci-custom :

 . venv/bin/activate
  script/ci-custom.py
  script/build_codeowners.py --check
  shell: /usr/bin/bash -e {0}
  env:
    DEFAULT_PYTHON: 3.9
    PYUPGRADE_TARGET: --py39-plus
    pythonLocation: /opt/hostedtoolcache/Python/3.9.19/x64
    PKG_CONFIG_PATH: /opt/hostedtoolcache/Python/3.9.19/x64/lib/pkgconfig
    Python_ROOT_DIR: /opt/hostedtoolcache/Python/3.9.19/x64
    Python2_ROOT_DIR: /opt/hostedtoolcache/Python/3.9.19/x64
    Python3_ROOT_DIR: /opt/hostedtoolcache/Python/3.9.19/x64
    LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.9.19/x64/lib
CODEOWNERS file is not up to date.
Please run `script/build_codeowners.py`
Error: Process completed with exit code 1.

alainstark avatar Apr 26 '24 21:04 alainstark

It's fine

nagyrobi avatar Apr 27 '24 05:04 nagyrobi

Hello, Is there anything left to do or should I just wait ?

alainstark avatar Apr 30 '24 21:04 alainstark

Just wait patiently from now...

nagyrobi avatar May 01 '24 05:05 nagyrobi

Feature request #621 is about the same device. Comment just to track.

szogun1987 avatar Oct 08 '24 09:10 szogun1987