goToMain / libosdp

Implementation of IEC 60839-11-5 OSDP (Open Supervised Device Protocol); provides a C library with support for C++, Rust and Python3
https://libosdp.sidcha.dev
Apache License 2.0
128 stars 69 forks source link

Add support for PD online status #135

Closed sidcha closed 9 months ago

sidcha commented 9 months ago

Also add tests for the added PD status.

sidcha commented 9 months ago

Fixes: #133

rm5248 commented 9 months ago

This looks good to me.

I would recommend adding the following to test_status.py to ensure that things work properly when the CP goes away:

def test_pd_status_afer_connection():
    cp.stop()
    time.sleep(1)
    assert pd.is_online() == False
    cp.start()
    assert cp.sc_wait(pd.address)
    assert pd.is_online()
    cp.stop()
    time.sleep(1)
    assert pd.is_online() == False
    cp.start() # ensure CP is running for proper tear-down
sidcha commented 9 months ago

Good catch, thanks for the review. Will make the change and merge it.

sidcha commented 9 months ago

This looks good to me.

I would recommend adding the following to test_status.py to ensure that things work properly when the CP goes away:

def test_pd_status_afer_connection():
    cp.stop()
    time.sleep(1)
    assert pd.is_online() == False
    cp.start()
    assert cp.sc_wait(pd.address)
    assert pd.is_online()
    cp.stop()
    time.sleep(1)
    assert pd.is_online() == False
    cp.start() # ensure CP is running for proper tear-down

Okay, I thought the last line with comment "ensure CP is running for proper tear-down" was what I missed. But now I see that you are suggesting a new test. This case already covered in other tests because at the point of test module entry, CP and PD is already running and SC active. I will merge this PR as is.