Skip to content

Iot Integration in ping #196

Closed
sivakondri-CT wants to merge 2 commits intogreearb:masterfrom
goyalsaurabh06:ping_iot
Closed

Iot Integration in ping #196
sivakondri-CT wants to merge 2 commits intogreearb:masterfrom
goyalsaurabh06:ping_iot

Conversation

@sivakondri-CT
Copy link
Copy Markdown
Contributor

No description provided.

…nning the ping test

Signed-off-by: sivakondri-CT <kondru.sankar@candelatech.com>
Copy link
Copy Markdown
Collaborator

@smileyrekiere smileyrekiere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May there be an example using the upstream port (See that the example goes to google.com, )

@sivakondri-CT
Copy link
Copy Markdown
Contributor Author

May there be an example using the upstream port (See that the example goes to google.com, )

VERIFIED CLI : python3 lf_interop_ping_plotter.py --mgr 192.168.207.78 --target 1.1.eth1 --real --ping_interval 5 --ping_duration 60s --use_default_config
I tried the above CLI using the upstream port, and it is working fine.

… ping report

VERIFIED CLI 1 : python3 lf_interop_ping_plotter.py --mgr 192.168.207.78 \
                                                    --target www.google.com \
                                                    --real --ping_interval 5 \
                                                    --ping_duration 60s \
                                                    --use_default_config \
                                                    --iot_test \
                                                    --iot_testname "Pingiot"
VERIFIED CLI 2 : python3 lf_interop_ping_plotter.py --mgr 192.168.207.78 \
                                                    --target www.google.com \
                                                    --real --ping_interval 5 \
                                                    --ping_duration 60s \
                                                    --use_default_config

Signed-off-by: sivakondri-CT <kondru.sankar@candelatech.com>
Copy link
Copy Markdown
Collaborator

@memnochproxy memnochproxy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not copy-paste whole methods all over the codebase. That make maintenance take a lot longer, and does not follow the DNRY rule. https://en.wikipedia.org/wiki/Don%27t_repeat_yourself

LFCliBase::parse_time

exit(1)


def duration_to_seconds(duration: str) -> int:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace calls to this with LFCliBase::parse_time

thread = threading.Thread(target=trigger_iot, args=(iot_ip, iot_port, iot_iterations, iot_delay, iot_device_list, iot_testname, iot_increment))
thread.start()
else:
total_secs = duration_to_seconds(args.ping_duration)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LFCliBase::parse_time

@sivakondri-CT
Copy link
Copy Markdown
Contributor Author

Please do not copy-paste whole methods all over the codebase. That make maintenance take a lot longer, and does not follow the DNRY rule. https://en.wikipedia.org/wiki/Don%27t_repeat_yourself

LFCliBase::parse_time

Hi @memnochproxy,Thanks for the reminder.For the next changes I’ll make sure to move common methods into a shared class and reuse them to avoid duplicated logic and I’ve updated the PR to use LFCliBase::parse_time

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.

3 participants