Skip to content

Add EVSE certificate injection and configuration file#159

Open
lho-stx wants to merge 17 commits intoSevenstax:masterfrom
lho-stx:add_evse_cert
Open

Add EVSE certificate injection and configuration file#159
lho-stx wants to merge 17 commits intoSevenstax:masterfrom
lho-stx:add_evse_cert

Conversation

@lho-stx
Copy link
Contributor

@lho-stx lho-stx commented Mar 8, 2023

No description provided.

@lho-stx lho-stx requested a review from jpo-stx March 8, 2023 13:49
Copy link
Contributor

@jpo-stx jpo-stx left a comment

Choose a reason for hiding this comment

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

Please rework the open points.

@lho-stx lho-stx requested a review from jpo-stx August 2, 2023 07:56
@lho-stx lho-stx requested a review from jpo-stx August 7, 2023 08:40
@lho-stx
Copy link
Contributor Author

lho-stx commented Sep 18, 2023

@jpo-stx all done, please review and merge

Application.py Outdated
parser.add_argument('-r', '--role', type=str, choices=('EVSE', 'EV'), required=True, help='This is the role of the Whitebeet. "EV" for EV mode and "EVSE" for EVSE mode')
parser.add_argument('-c', '--config', type=str, help='Path to configuration file. Defaults to ./ev.json.\nA MAC present in the config file will override a MAC provided with -m argument.', nargs='?', const="./ev.json")
parser.add_argument('-r', '--role', type=str, choices=('EV', 'EVSE'), help='This is the role of the Whitebeet. "EV" for EV mode and "EVSE" for EVSE mode')
parser.add_argument('-c', '--config', type=str, help='Path to configuration file. Defaults to ./config.json.\nA MAC present in the config file will override a MAC provided with -m argument.', nargs='?', const="./config.json")
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer that command line arguments take precedence over configuartion files. This is more common for most applications.

parser.add_argument('interface_type', type=str, choices=('eth', 'spi'), help='Type of the interface through which the Whitebeet is connected. ("eth" or "spi").')
parser.add_argument('-i', '--interface', type=str, required=True, help='This is the name of the interface where the Whitebeet is connected to (i.e. for eth "eth0" or spi "0").')
parser.add_argument('-m', '--mac', type=str, help='This is the MAC address of the ethernet interface of the Whitebeet (i.e. "{}").'.format(WHITEBBET_DEFAULT_MAC))
parser.add_argument('-r', '--role', type=str, choices=('EVSE', 'EV'), required=True, help='This is the role of the Whitebeet. "EV" for EV mode and "EVSE" for EVSE mode')
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the required=True was removed unintentionally. The code checks with if(args.role == "EV"): and elif(args.role == 'EVSE'):, if the role parameter missing nothing will happen here and the application just exit.

Application.py Outdated
evse.loop()
print("EVSE loop finished")
#set the Whitebeet time
#evse.setTime()
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should not be commented out, we need the time for checking the injected certificates.

print("##############################################")
print("")
print("")
time.sleep(4)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we wait here?

print("Injecting Certificates!")

#print("EVSE OPEN OWN PATH")
#evse.openOwn()
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this commented out?

print("##########################")
self.addOwn()

time.sleep(2)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we wait here?

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