Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for PD online status #135

Merged
merged 6 commits into from
Oct 11, 2023
Merged

Add support for PD online status #135

merged 6 commits into from
Oct 11, 2023

Conversation

sidcha
Copy link
Member

@sidcha sidcha commented Oct 9, 2023

Also add tests for the added PD status.

@sidcha
Copy link
Member Author

sidcha commented Oct 9, 2023

Fixes: #133

PD mode always returns online state for PD because PD does not have any
state. But some users pefer to use the last poll time stamp as an
indicator for PD online/offline status. Let us fix the code to do just
that.

Fixes: #133
Signed-off-by: Siddharth Chandrasekaran <[email protected]>
Signed-off-by: Siddharth Chandrasekaran <[email protected]>
Signed-off-by: Siddharth Chandrasekaran <[email protected]>
Signed-off-by: Siddharth Chandrasekaran <[email protected]>
@rm5248
Copy link

rm5248 commented Oct 9, 2023

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
Copy link
Member Author

sidcha commented Oct 9, 2023

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

@sidcha
Copy link
Member Author

sidcha commented Oct 11, 2023

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.

@sidcha sidcha merged commit e4df2be into master Oct 11, 2023
7 of 8 checks passed
@sidcha sidcha deleted the osdp_pd_status branch November 1, 2023 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants