I’ve had lwip pppos (via SIMCOM) running on my desk overnight, and it seems stable. Of course, we’ll need to do some fine tuning based on feedback in the field, but at least PPP connections can be brought up and down cleanly, and that is the base capability needed for more sophistication to be built on top. The OVMS v2 client code also seems good. With DEMO car, it is not producing too much data, and seems roughly comparable to v2 usage. For example, here is the start of one of my recent sessions (log level info): I (17600) simcom: State: Enter PoweringOn state I (17600) simcom: Power Cycle I (23660) simcom: State: Enter PoweredOn state I (42770) simcom: State: Enter MuxStart state I (42770) gsm-mux: Start MUX I (42780) gsm-mux: Channel #0 is open I (42780) gsm-mux: Channel #1 is open I (42790) gsm-mux: Channel #2 is open I (42800) gsm-mux: Channel #3 is open I (42810) gsm-mux: Channel #4 is open I (43750) simcom: State: Enter NetWait state I (53830) simcom: CREG Network Registration: RegisteredRoaming I (54750) simcom: State: Enter NetStart state I (55890) simcom: PPP Connection is ready to start I (56750) simcom: State: Enter NetMode state I (56750) gsm-ppp: Initialising... I (59770) gsm-ppp: StatusCallBack: None I (59770) gsm-ppp: status_cb: Connected I (59770) gsm-ppp: our_ipaddr = 10.52.223.8 I (59770) gsm-ppp: his_ipaddr = 10.64.64.64 I (59770) gsm-ppp: netmask = 255.255.255.255 I (59770) gsm-ppp: our6_ipaddr = :: I (62570) ovms-server-v2: Status: Network connectivity established I (62570) ovms-server-v2: Connection is api.openvehicles.com:6867 TESTCAR/... I (63430) ovms-server-v2: Connected to OVMS Server V2 at api.openvehicles.com I (63430) ovms-server-v2: Status: Connected to server I (63440) ovms-server-v2: Status: Logging in... I (63440) ovms-server-v2: Sending server login: MP-C 0 ... ... TESTCAR I (64030) ovms-server-v2: Received welcome response MP-S 0 ... ... I (64030) ovms-server-v2: Got server response: MP-S 0 ... ... I (64040) ovms-server-v2: Server token is ... and digest is ... I (64040) ovms-server-v2: Status: Server auth ok. Now priming crypto. I (64040) ovms-server-v2: Shared secret key is .../... (44 bytes) I (64040) ovms-server-v2: Status: OVMS V2 login successful, and crypto channel established I (64040) ovms-server-v2: Status: Connected and logged in I (64050) ovms-server-v2: Send MP-0 S50,K,0,0,done,standard,200,160,0,0,0,0,13,4,0,0,0,0,160.00,0,0,0,0,-1,0,0,0,0,0,400,0,0.00,400.00,100 I (64050) ovms-server-v2: Send MP-0 D0,0,5,22,30,25,0,1000000,0,61,22,0,0,0,0,0,0,0,22,0 I (64060) ovms-server-v2: Send MP-0 L22.280869,114.160599,10,30,1,0,0,0,0.0,0,0 I (64070) ovms-server-v2: Send MP-0 W30,33,40,34,30,33,40,34,0 I (64080) ovms-server-v2: Send MP-0 F3.0.0/factory/main build (idf v2.1.1-8-g5ae2c0d0) Dec 12 2017 07:55:27,DEMODEMODEMO,9,1,DEMO,CMHK Jsy Tel I (64100) ovms-server-v2: Incoming Msg: MP-0 Z0 I (665100) ovms-server-v2: Send MP-0 S50,K,0,0,done,standard,200,160,0,0,0,0,13,4,0,0,0,0,160.00,0,0,0,0,-1,0,0,0,0,0,400,0,0.00,400.00,100 I (665110) ovms-server-v2: Send MP-0 D0,0,5,22,30,25,0,1000000,0,662,22,0,0,0,0,0,0,0,22,0 I (665120) ovms-server-v2: Send MP-0 F3.0.0/factory/main build (idf v2.1.1-8-g5ae2c0d0) Dec 12 2017 07:55:27,DEMODEMODEMO,7,1,DEMO,CMHK Jsy Tel OVMS > server v2 status Connected and logged in I (1266130) ovms-server-v2: Send MP-0 S50,K,0,0,done,standard,200,160,0,0,0,0,13,4,0,0,0,0,160.00,0,0,0,0,-1,0,0,0,0,0,400,0,0.00,400.00,100 I (1266140) ovms-server-v2: Send MP-0 D0,0,5,22,30,25,0,1000000,0,1263,22,0,0,0,0,0,0,0,22,0 I (1266150) ovms-server-v2: Send MP-0 F3.0.0/factory/main build (idf v2.1.1-8-g5ae2c0d0) Dec 12 2017 07:55:27,DEMODEMODEMO,8,1,DEMO,CMHK Jsy Tel OVMS > charge start Charge has been started I (1744160) ovms-server-v2: Send MP-0 S50,K,220,32,charging,standard,200,160,0,0,0,0,3,1,0,0,0,0,160.00,0,0,0,0,-1,0,0,0,0,0,400,0,-0.00,400.00,100 I (1744170) ovms-server-v2: Send MP-0 D28,0,5,22,30,25,0,1000000,0,1741,22,0,0,0,0,0,0,0,22,0 I (1867180) ovms-server-v2: Send MP-0 S51,K,220,32,charging,standard,200,160,0,2,0,0,3,1,0,0,0,0,160.00,0,0,0,0,-1,0,0,0,0,0,400,0,-0.00,400.00,100 I (1867190) ovms-server-v2: Send MP-0 D28,0,5,22,30,25,0,1000000,0,1864,22,0,0,0,0,0,0,0,22,0 I (2468200) ovms-server-v2: Send MP-0 S57,K,220,32,charging,standard,200,160,0,12,0,0,3,1,0,0,0,0,160.00,0,0,0,0,-1,0,0,0,0,0,400,0,-0.00,400.00,100 I (2468210) ovms-server-v2: Send MP-0 D28,0,5,22,30,25,0,1000000,0,2465,22,0,0,0,0,0,0,0,22,0 I (3069220) ovms-server-v2: Send MP-0 S63,K,220,32,charging,standard,200,160,0,22,0,0,3,1,0,0,0,0,160.00,0,0,0,0,-1,0,0,0,0,0,400,0,-0.00,400.00,100 I (3069230) ovms-server-v2: Send MP-0 D28,0,5,22,30,25,0,1000000,0,3066,22,0,0,0,0,0,0,0,22,0 I (3670240) ovms-server-v2: Send MP-0 S69,K,220,32,charging,standard,200,160,0,32,0,0,3,1,0,0,0,0,160.00,0,0,0,0,-1,0,0,0,0,0,400,0,-0.00,400.00,100 I (3670250) ovms-server-v2: Send MP-0 D28,0,5,22,30,25,0,1000000,0,3667,22,0,0,0,0,0,0,0,22,0 That is one set of messages every 10 minutes (the numbers in brackets after the “I” seem to be millisecond timestamps). So, now to think about how to manage this in general. To make it easier for the end user. Here are some notes/thoughts: We continue to use events and scripting for fine-tuned control. For configuration, we offer options to set values and have those automatically actioned at startup. Examples: If a vehicle type is configured, then automatically start a vehicle module of that type. If a v2 configuration is present, then automatically start a ovms server v2 connection. For networking, we have the network manager component. The idea here is to extend that to provide network link logic. Examples: If wifi comes up, then sleep the simcom modem If wifi goes down, then wake up the simcom modem The above configuration settings can be simply mapped to v2 protocol features/parameters. While Config parameters are currently registered and visible (MyConfig.RegisterParam), instances are not. That makes it tricky to see what config instances are available. In some of the code, we document the instances as comments after the parameter is registered. My suggestion here is to add a MyConfig.RegisterInstance to allow for instance registration (with a default value). If the instance does not exist, then it would be created with the value set as per the default. That would then make it visible to the user. The only downside to this is that the default value can’t later be changed. What do people think? Does that make sense as an approach? Implementation should not be difficult. Regards, Mark.
I think the approach you propose sounds like a nice solution. BTW: I’ve sent a pull request with the latest version of Kia Soul module. Geir
12. des. 2017 kl. 02:29 skrev Mark Webb-Johnson <mark@webb-johnson.net>:
I’ve had lwip pppos (via SIMCOM) running on my desk overnight, and it seems stable. Of course, we’ll need to do some fine tuning based on feedback in the field, but at least PPP connections can be brought up and down cleanly, and that is the base capability needed for more sophistication to be built on top.
The OVMS v2 client code also seems good. With DEMO car, it is not producing too much data, and seems roughly comparable to v2 usage.
For example, here is the start of one of my recent sessions (log level info):
I (17600) simcom: State: Enter PoweringOn state I (17600) simcom: Power Cycle I (23660) simcom: State: Enter PoweredOn state I (42770) simcom: State: Enter MuxStart state I (42770) gsm-mux: Start MUX I (42780) gsm-mux: Channel #0 is open I (42780) gsm-mux: Channel #1 is open I (42790) gsm-mux: Channel #2 is open I (42800) gsm-mux: Channel #3 is open I (42810) gsm-mux: Channel #4 is open I (43750) simcom: State: Enter NetWait state I (53830) simcom: CREG Network Registration: RegisteredRoaming I (54750) simcom: State: Enter NetStart state I (55890) simcom: PPP Connection is ready to start I (56750) simcom: State: Enter NetMode state I (56750) gsm-ppp: Initialising... I (59770) gsm-ppp: StatusCallBack: None I (59770) gsm-ppp: status_cb: Connected I (59770) gsm-ppp: our_ipaddr = 10.52.223.8 I (59770) gsm-ppp: his_ipaddr = 10.64.64.64 I (59770) gsm-ppp: netmask = 255.255.255.255 I (59770) gsm-ppp: our6_ipaddr = :: I (62570) ovms-server-v2: Status: Network connectivity established I (62570) ovms-server-v2: Connection is api.openvehicles.com:6867 <http://api.openvehicles.com:6867/> TESTCAR/... I (63430) ovms-server-v2: Connected to OVMS Server V2 at api.openvehicles.com <http://api.openvehicles.com/> I (63430) ovms-server-v2: Status: Connected to server I (63440) ovms-server-v2: Status: Logging in... I (63440) ovms-server-v2: Sending server login: MP-C 0 ... ... TESTCAR I (64030) ovms-server-v2: Received welcome response MP-S 0 ... ... I (64030) ovms-server-v2: Got server response: MP-S 0 ... ... I (64040) ovms-server-v2: Server token is ... and digest is ... I (64040) ovms-server-v2: Status: Server auth ok. Now priming crypto. I (64040) ovms-server-v2: Shared secret key is .../... (44 bytes) I (64040) ovms-server-v2: Status: OVMS V2 login successful, and crypto channel established I (64040) ovms-server-v2: Status: Connected and logged in I (64050) ovms-server-v2: Send MP-0 S50,K,0,0,done,standard,200,160,0,0,0,0,13,4,0,0,0,0,160.00,0,0,0,0,-1,0,0,0,0,0,400,0,0.00,400.00,100 I (64050) ovms-server-v2: Send MP-0 D0,0,5,22,30,25,0,1000000,0,61,22,0,0,0,0,0,0,0,22,0 I (64060) ovms-server-v2: Send MP-0 L22.280869,114.160599,10,30,1,0,0,0,0.0,0,0 I (64070) ovms-server-v2: Send MP-0 W30,33,40,34,30,33,40,34,0 I (64080) ovms-server-v2: Send MP-0 F3.0.0/factory/main build (idf v2.1.1-8-g5ae2c0d0) Dec 12 2017 07:55:27,DEMODEMODEMO,9,1,DEMO,CMHK Jsy Tel I (64100) ovms-server-v2: Incoming Msg: MP-0 Z0 I (665100) ovms-server-v2: Send MP-0 S50,K,0,0,done,standard,200,160,0,0,0,0,13,4,0,0,0,0,160.00,0,0,0,0,-1,0,0,0,0,0,400,0,0.00,400.00,100 I (665110) ovms-server-v2: Send MP-0 D0,0,5,22,30,25,0,1000000,0,662,22,0,0,0,0,0,0,0,22,0 I (665120) ovms-server-v2: Send MP-0 F3.0.0/factory/main build (idf v2.1.1-8-g5ae2c0d0) Dec 12 2017 07:55:27,DEMODEMODEMO,7,1,DEMO,CMHK Jsy Tel
OVMS > server v2 status Connected and logged in
I (1266130) ovms-server-v2: Send MP-0 S50,K,0,0,done,standard,200,160,0,0,0,0,13,4,0,0,0,0,160.00,0,0,0,0,-1,0,0,0,0,0,400,0,0.00,400.00,100 I (1266140) ovms-server-v2: Send MP-0 D0,0,5,22,30,25,0,1000000,0,1263,22,0,0,0,0,0,0,0,22,0 I (1266150) ovms-server-v2: Send MP-0 F3.0.0/factory/main build (idf v2.1.1-8-g5ae2c0d0) Dec 12 2017 07:55:27,DEMODEMODEMO,8,1,DEMO,CMHK Jsy Tel
OVMS > charge start Charge has been started I (1744160) ovms-server-v2: Send MP-0 S50,K,220,32,charging,standard,200,160,0,0,0,0,3,1,0,0,0,0,160.00,0,0,0,0,-1,0,0,0,0,0,400,0,-0.00,400.00,100 I (1744170) ovms-server-v2: Send MP-0 D28,0,5,22,30,25,0,1000000,0,1741,22,0,0,0,0,0,0,0,22,0 I (1867180) ovms-server-v2: Send MP-0 S51,K,220,32,charging,standard,200,160,0,2,0,0,3,1,0,0,0,0,160.00,0,0,0,0,-1,0,0,0,0,0,400,0,-0.00,400.00,100 I (1867190) ovms-server-v2: Send MP-0 D28,0,5,22,30,25,0,1000000,0,1864,22,0,0,0,0,0,0,0,22,0 I (2468200) ovms-server-v2: Send MP-0 S57,K,220,32,charging,standard,200,160,0,12,0,0,3,1,0,0,0,0,160.00,0,0,0,0,-1,0,0,0,0,0,400,0,-0.00,400.00,100 I (2468210) ovms-server-v2: Send MP-0 D28,0,5,22,30,25,0,1000000,0,2465,22,0,0,0,0,0,0,0,22,0 I (3069220) ovms-server-v2: Send MP-0 S63,K,220,32,charging,standard,200,160,0,22,0,0,3,1,0,0,0,0,160.00,0,0,0,0,-1,0,0,0,0,0,400,0,-0.00,400.00,100 I (3069230) ovms-server-v2: Send MP-0 D28,0,5,22,30,25,0,1000000,0,3066,22,0,0,0,0,0,0,0,22,0 I (3670240) ovms-server-v2: Send MP-0 S69,K,220,32,charging,standard,200,160,0,32,0,0,3,1,0,0,0,0,160.00,0,0,0,0,-1,0,0,0,0,0,400,0,-0.00,400.00,100 I (3670250) ovms-server-v2: Send MP-0 D28,0,5,22,30,25,0,1000000,0,3667,22,0,0,0,0,0,0,0,22,0
That is one set of messages every 10 minutes (the numbers in brackets after the “I” seem to be millisecond timestamps).
So, now to think about how to manage this in general. To make it easier for the end user. Here are some notes/thoughts:
We continue to use events and scripting for fine-tuned control.
For configuration, we offer options to set values and have those automatically actioned at startup. Examples: If a vehicle type is configured, then automatically start a vehicle module of that type. If a v2 configuration is present, then automatically start a ovms server v2 connection.
For networking, we have the network manager component. The idea here is to extend that to provide network link logic. Examples: If wifi comes up, then sleep the simcom modem If wifi goes down, then wake up the simcom modem
The above configuration settings can be simply mapped to v2 protocol features/parameters.
While Config parameters are currently registered and visible (MyConfig.RegisterParam), instances are not. That makes it tricky to see what config instances are available. In some of the code, we document the instances as comments after the parameter is registered. My suggestion here is to add a MyConfig.RegisterInstance to allow for instance registration (with a default value). If the instance does not exist, then it would be created with the value set as per the default. That would then make it visible to the user. The only downside to this is that the default value can’t later be changed.
What do people think? Does that make sense as an approach? Implementation should not be difficult.
Regards, Mark.
_______________________________________________ OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
Am 12.12.2017 um 02:29 schrieb Mark Webb-Johnson:
I’ve had lwip pppos (via SIMCOM) running on my desk overnight, and it seems stable. Of course, we’ll need to do some fine tuning based on feedback in the field, but at least PPP connections can be brought up and down cleanly, and that is the base capability needed for more sophistication to be built on top.
I did the same test since yesterday, but my ppp connection got lost somewhere and couldn't be reestablished, even after power simcom off / on. I missed keeping my PC on, so I've lost the critical log output. The last state was a loop: I (81512562) simcom: State: Enter NetStart state I (81541562) simcom: State timeout, transition to 7 I (81541562) simcom: State: Enter NetLoss state I (81541562) gsm-ppp: Shutting down (hard)... I (81541562) gsm-ppp: StatusCallBack: User Interrupt E (81541562) gsm-ppp: status_cb: User interrupt| I (81541562) gsm-ppp: Shutdown (via status callback) I (81541562) events: Signal(system.modem.down) I (81551562) simcom: State timeout, transition to 5 I (81551562) simcom: State: Enter NetWait state I (81551562) gsm-nmea: Startup I (81555562) simcom: State: Enter NetStart state I (81584562) simcom: State timeout, transition to 7 I (81584562) simcom: State: Enter NetLoss state I (81584562) gsm-ppp: Shutting down (hard)... I (81584562) gsm-ppp: StatusCallBack: User Interrupt E (81584562) gsm-ppp: status_cb: User interrupt| I (81584562) gsm-ppp: Shutdown (via status callback) I (81584562) events: Signal(system.modem.down) I (81594562) simcom: State timeout, transition to 5 I (81594562) simcom: State: Enter NetWait state I (81594562) gsm-nmea: Startup I (81598562) simcom: State: Enter NetStart state I (81627562) simcom: State timeout, transition to 7 I (81627562) simcom: State: Enter NetLoss state I (81627562) gsm-ppp: Shutting down (hard)... I (81627562) gsm-ppp: StatusCallBack: User Interrupt E (81627562) gsm-ppp: status_cb: User interrupt| I (81627562) gsm-ppp: Shutdown (via status callback) I (81627562) events: Signal(system.modem.down) I (81637562) simcom: State timeout, transition to 5 I (81637562) simcom: State: Enter NetWait state I (81637562) gsm-nmea: Startup I (81641562) simcom: State: Enter NetStart state I (81670562) simcom: State timeout, transition to 7 I (81670562) simcom: State: Enter NetLoss state I (81670562) gsm-ppp: Shutting down (hard)... I (81670562) gsm-ppp: StatusCallBack: User Interrupt E (81670562) gsm-ppp: status_cb: User interrupt| I (81670562) gsm-ppp: Shutdown (via status callback) I (81670562) events: Signal(system.modem.down) I (81680562) simcom: State timeout, transition to 5 I (81680562) simcom: State: Enter NetWait state I (81680562) gsm-nmea: Startup … and so on.
So, now to think about how to manage this in general. To make it easier for the end user. Here are some notes/thoughts:
1. We continue to use events and scripting for fine-tuned control.
2. For configuration, we offer options to set values and have those automatically actioned at startup. Examples: * If a vehicle type is configured, then automatically start a vehicle module of that type. * If a v2 configuration is present, then automatically start a ovms server v2 connection.
3. For networking, we have the network manager component. The idea here is to extend that to provide network link logic. Examples: * If wifi comes up, then sleep the simcom modem * If wifi goes down, then wake up the simcom modem
4. The above configuration settings can be simply mapped to v2 protocol features/parameters.
5. While Config parameters are currently registered and visible (MyConfig.RegisterParam), instances are not. That makes it tricky to see what config instances are available. In some of the code, we document the instances as comments after the parameter is registered. My suggestion here is to add a MyConfig.RegisterInstance to allow for instance registration (with a default value). If the instance does not exist, then it would be created with the value set as per the default. That would then make it visible to the user. The only downside to this is that the default value can’t later be changed.
What do people think? Does that make sense as an approach? Implementation should not be difficult.
Re #2: yes, but we need some simple way to prevent the auto start, as the module may crash before interaction is possible. Re #5: definitely, that may also allow tab command expansion down to instances. Btw: I've always registered my config "x.rt" as readable, but "config list x.rt" will no longer show any values. I'm pretty sure it did before. Regards, Michael -- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
Am 12.12.2017 um 20:12 schrieb Michael Balzer:
Btw: I've always registered my config "x.rt" as readable, but "config list x.rt" will no longer show any values. I'm pretty sure it did before.
Found & fixed, the access rights wouldn't be restored on loading the config. Regards, Michael -- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
Michael, I don't think this is correct/necessary. The access rights are provided in the Register command, when the parameter is registered. This is done each and every time. We don't need to persist this to /store file system. There was an issue with load order - where parameters could be loaded before the Register function call. Perhaps that is what is causing the bug you are seeing? The code was fixed to work around this, but perhaps it is not setting the access rights correctly in that case? Regards, Mark
On 13 Dec 2017, at 3:32 AM, Michael Balzer <dexter@expeedo.de> wrote:
Am 12.12.2017 um 20:12 schrieb Michael Balzer: Btw: I've always registered my config "x.rt" as readable, but "config list x.rt" will no longer show any values. I'm pretty sure it did before.
Found & fixed, the access rights wouldn't be restored on loading the config.
Regards, Michael
-- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
_______________________________________________ OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
I've pushed two commits regarding this, the first only fixed the primary problem of not setting the access rights again on RegisterParam() after loading the config cache. The second one adds storage persistance, as I think it's OK to persist this info. The RegisterParam() call will always set the current access rights as programmed, this is only meant to restore the latest access rights of parameters registered by loadable modules like vehicles. So I think this should be OK? Regards, Michael Am 12.12.2017 um 22:45 schrieb Mark Webb-Johnson:
Michael,
I don't think this is correct/necessary.
The access rights are provided in the Register command, when the parameter is registered. This is done each and every time. We don't need to persist this to /store file system.
There was an issue with load order - where parameters could be loaded before the Register function call. Perhaps that is what is causing the bug you are seeing? The code was fixed to work around this, but perhaps it is not setting the access rights correctly in that case?
Regards, Mark
On 13 Dec 2017, at 3:32 AM, Michael Balzer <dexter@expeedo.de> wrote:
Am 12.12.2017 um 20:12 schrieb Michael Balzer: Btw: I've always registered my config "x.rt" as readable, but "config list x.rt" will no longer show any values. I'm pretty sure it did before. Found & fixed, the access rights wouldn't be restored on loading the config.
Regards, Michael
-- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
_______________________________________________ OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
-- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
It is the second one I am not sure is necessary. If access is controlled by the Register, why persist at all? What would happen if the permissions in Register were changed? What would happen if an instance "#access" was set? Regards, Mark
On 13 Dec 2017, at 5:54 AM, Michael Balzer <dexter@expeedo.de> wrote:
I've pushed two commits regarding this, the first only fixed the primary problem of not setting the access rights again on RegisterParam() after loading the config cache.
The second one adds storage persistance, as I think it's OK to persist this info. The RegisterParam() call will always set the current access rights as programmed, this is only meant to restore the latest access rights of parameters registered by loadable modules like vehicles.
So I think this should be OK?
Regards, Michael
Am 12.12.2017 um 22:45 schrieb Mark Webb-Johnson: Michael,
I don't think this is correct/necessary.
The access rights are provided in the Register command, when the parameter is registered. This is done each and every time. We don't need to persist this to /store file system.
There was an issue with load order - where parameters could be loaded before the Register function call. Perhaps that is what is causing the bug you are seeing? The code was fixed to work around this, but perhaps it is not setting the access rights correctly in that case?
Regards, Mark
On 13 Dec 2017, at 3:32 AM, Michael Balzer <dexter@expeedo.de> wrote:
Am 12.12.2017 um 20:12 schrieb Michael Balzer: Btw: I've always registered my config "x.rt" as readable, but "config list x.rt" will no longer show any values. I'm pretty sure it did before. Found & fixed, the access rights wouldn't be restored on loading the config.
Regards, Michael
-- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
_______________________________________________ OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
-- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
_______________________________________________ OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
It's necessary if we want to be able to see configs of not yet loaded modules, i.e. vehicles. Without the persistance the configs are not readable before the module is actually loaded by the user -- so a module configuration can also not be checked without loading the module. That was my reason to change this: without persistance, I cannot see my Twizy configuration after boot before issueing "vehicle module RT". If a part of my config needs to be changed before starting the module, I cannot see what I've set before. You're right in the special meaning of instance names prefixed with "#" -- these should not be allowed for this serialization form. I chose "#" as that's a typical comment line character, and I thought names like this will normally not be used. I'll add a check to prevent using instance names colliding with that pattern, if you agree on the basic solution. Regards, Michael Am 12.12.2017 um 23:19 schrieb Mark Webb-Johnson:
It is the second one I am not sure is necessary.
If access is controlled by the Register, why persist at all? What would happen if the permissions in Register were changed? What would happen if an instance "#access" was set?
Regards, Mark
On 13 Dec 2017, at 5:54 AM, Michael Balzer <dexter@expeedo.de> wrote:
I've pushed two commits regarding this, the first only fixed the primary problem of not setting the access rights again on RegisterParam() after loading the config cache.
The second one adds storage persistance, as I think it's OK to persist this info. The RegisterParam() call will always set the current access rights as programmed, this is only meant to restore the latest access rights of parameters registered by loadable modules like vehicles.
So I think this should be OK?
Regards, Michael
Am 12.12.2017 um 22:45 schrieb Mark Webb-Johnson: Michael,
I don't think this is correct/necessary.
The access rights are provided in the Register command, when the parameter is registered. This is done each and every time. We don't need to persist this to /store file system.
There was an issue with load order - where parameters could be loaded before the Register function call. Perhaps that is what is causing the bug you are seeing? The code was fixed to work around this, but perhaps it is not setting the access rights correctly in that case?
Regards, Mark
On 13 Dec 2017, at 3:32 AM, Michael Balzer <dexter@expeedo.de> wrote:
Am 12.12.2017 um 20:12 schrieb Michael Balzer: Btw: I've always registered my config "x.rt" as readable, but "config list x.rt" will no longer show any values. I'm pretty sure it did before. Found & fixed, the access rights wouldn't be restored on loading the config.
Regards, Michael
-- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
_______________________________________________ OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev -- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
_______________________________________________ OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
-- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
Another option would be to do the RegisterParam() call in the framework registration class, .i.e. OvmsVehicleRenaultTwizyInit. This would assure access rights are set on boot if the module is included, regardless of it being actually started. That smells a bit like breaking the factory class pattern, but seems to be an alternative. Regards, Michael Am 12.12.2017 um 23:35 schrieb Michael Balzer:
It's necessary if we want to be able to see configs of not yet loaded modules, i.e. vehicles. Without the persistance the configs are not readable before the module is actually loaded by the user -- so a module configuration can also not be checked without loading the module.
That was my reason to change this: without persistance, I cannot see my Twizy configuration after boot before issueing "vehicle module RT". If a part of my config needs to be changed before starting the module, I cannot see what I've set before.
You're right in the special meaning of instance names prefixed with "#" -- these should not be allowed for this serialization form. I chose "#" as that's a typical comment line character, and I thought names like this will normally not be used. I'll add a check to prevent using instance names colliding with that pattern, if you agree on the basic solution.
Regards, Michael
Am 12.12.2017 um 23:19 schrieb Mark Webb-Johnson:
It is the second one I am not sure is necessary.
If access is controlled by the Register, why persist at all? What would happen if the permissions in Register were changed? What would happen if an instance "#access" was set?
Regards, Mark
On 13 Dec 2017, at 5:54 AM, Michael Balzer <dexter@expeedo.de> wrote:
I've pushed two commits regarding this, the first only fixed the primary problem of not setting the access rights again on RegisterParam() after loading the config cache.
The second one adds storage persistance, as I think it's OK to persist this info. The RegisterParam() call will always set the current access rights as programmed, this is only meant to restore the latest access rights of parameters registered by loadable modules like vehicles.
So I think this should be OK?
Regards, Michael
Am 12.12.2017 um 22:45 schrieb Mark Webb-Johnson: Michael,
I don't think this is correct/necessary.
The access rights are provided in the Register command, when the parameter is registered. This is done each and every time. We don't need to persist this to /store file system.
There was an issue with load order - where parameters could be loaded before the Register function call. Perhaps that is what is causing the bug you are seeing? The code was fixed to work around this, but perhaps it is not setting the access rights correctly in that case?
Regards, Mark
On 13 Dec 2017, at 3:32 AM, Michael Balzer <dexter@expeedo.de> wrote:
Am 12.12.2017 um 20:12 schrieb Michael Balzer: Btw: I've always registered my config "x.rt" as readable, but "config list x.rt" will no longer show any values. I'm pretty sure it did before. Found & fixed, the access rights wouldn't be restored on loading the config.
Regards, Michael
-- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
_______________________________________________ OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev -- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
_______________________________________________ OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
-- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
Oh, I see. Yes, Register in the class init was what I anticipated. So you could configure the vehicle before starting it. Same as OVMS server v2, for example. Parameters are registered even if you don’t start the module. Regards, Mark
On 13 Dec 2017, at 6:48 AM, Michael Balzer <dexter@expeedo.de> wrote:
Another option would be to do the RegisterParam() call in the framework registration class, .i.e. OvmsVehicleRenaultTwizyInit. This would assure access rights are set on boot if the module is included, regardless of it being actually started.
That smells a bit like breaking the factory class pattern, but seems to be an alternative.
Regards, Michael
Am 12.12.2017 um 23:35 schrieb Michael Balzer: It's necessary if we want to be able to see configs of not yet loaded modules, i.e. vehicles. Without the persistance the configs are not readable before the module is actually loaded by the user -- so a module configuration can also not be checked without loading the module.
That was my reason to change this: without persistance, I cannot see my Twizy configuration after boot before issueing "vehicle module RT". If a part of my config needs to be changed before starting the module, I cannot see what I've set before.
You're right in the special meaning of instance names prefixed with "#" -- these should not be allowed for this serialization form. I chose "#" as that's a typical comment line character, and I thought names like this will normally not be used. I'll add a check to prevent using instance names colliding with that pattern, if you agree on the basic solution.
Regards, Michael
Am 12.12.2017 um 23:19 schrieb Mark Webb-Johnson: It is the second one I am not sure is necessary.
If access is controlled by the Register, why persist at all? What would happen if the permissions in Register were changed? What would happen if an instance "#access" was set?
Regards, Mark
On 13 Dec 2017, at 5:54 AM, Michael Balzer <dexter@expeedo.de> wrote:
I've pushed two commits regarding this, the first only fixed the primary problem of not setting the access rights again on RegisterParam() after loading the config cache.
The second one adds storage persistance, as I think it's OK to persist this info. The RegisterParam() call will always set the current access rights as programmed, this is only meant to restore the latest access rights of parameters registered by loadable modules like vehicles.
So I think this should be OK?
Regards, Michael
Am 12.12.2017 um 22:45 schrieb Mark Webb-Johnson: Michael,
I don't think this is correct/necessary.
The access rights are provided in the Register command, when the parameter is registered. This is done each and every time. We don't need to persist this to /store file system.
There was an issue with load order - where parameters could be loaded before the Register function call. Perhaps that is what is causing the bug you are seeing? The code was fixed to work around this, but perhaps it is not setting the access rights correctly in that case?
Regards, Mark
On 13 Dec 2017, at 3:32 AM, Michael Balzer <dexter@expeedo.de> wrote:
> Am 12.12.2017 um 20:12 schrieb Michael Balzer: > Btw: I've always registered my config "x.rt" as readable, but "config list x.rt" will no longer show any values. I'm pretty sure it did before. Found & fixed, the access rights wouldn't be restored on loading the config.
Regards, Michael
-- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
_______________________________________________ OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev -- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
_______________________________________________ OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
-- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
_______________________________________________ OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
It's actually done both ways in other modules up to now, i.e. the simcom module does it in the Init class, the locations does it in the constructor. A disadvantage of doing it in the Init classes is, especially if we add instance defaults to the registration, all configs of all modules compiled in will always be listed, regardless of the user actually needing or using them. Regards, Michael Am 12.12.2017 um 23:48 schrieb Michael Balzer:
Another option would be to do the RegisterParam() call in the framework registration class, .i.e. OvmsVehicleRenaultTwizyInit. This would assure access rights are set on boot if the module is included, regardless of it being actually started.
That smells a bit like breaking the factory class pattern, but seems to be an alternative.
Regards, Michael
Am 12.12.2017 um 23:35 schrieb Michael Balzer:
It's necessary if we want to be able to see configs of not yet loaded modules, i.e. vehicles. Without the persistance the configs are not readable before the module is actually loaded by the user -- so a module configuration can also not be checked without loading the module.
That was my reason to change this: without persistance, I cannot see my Twizy configuration after boot before issueing "vehicle module RT". If a part of my config needs to be changed before starting the module, I cannot see what I've set before.
You're right in the special meaning of instance names prefixed with "#" -- these should not be allowed for this serialization form. I chose "#" as that's a typical comment line character, and I thought names like this will normally not be used. I'll add a check to prevent using instance names colliding with that pattern, if you agree on the basic solution.
Regards, Michael
Am 12.12.2017 um 23:19 schrieb Mark Webb-Johnson:
It is the second one I am not sure is necessary.
If access is controlled by the Register, why persist at all? What would happen if the permissions in Register were changed? What would happen if an instance "#access" was set?
Regards, Mark
On 13 Dec 2017, at 5:54 AM, Michael Balzer <dexter@expeedo.de> wrote:
I've pushed two commits regarding this, the first only fixed the primary problem of not setting the access rights again on RegisterParam() after loading the config cache.
The second one adds storage persistance, as I think it's OK to persist this info. The RegisterParam() call will always set the current access rights as programmed, this is only meant to restore the latest access rights of parameters registered by loadable modules like vehicles.
So I think this should be OK?
Regards, Michael
Am 12.12.2017 um 22:45 schrieb Mark Webb-Johnson: Michael,
I don't think this is correct/necessary.
The access rights are provided in the Register command, when the parameter is registered. This is done each and every time. We don't need to persist this to /store file system.
There was an issue with load order - where parameters could be loaded before the Register function call. Perhaps that is what is causing the bug you are seeing? The code was fixed to work around this, but perhaps it is not setting the access rights correctly in that case?
Regards, Mark
On 13 Dec 2017, at 3:32 AM, Michael Balzer <dexter@expeedo.de> wrote:
> Am 12.12.2017 um 20:12 schrieb Michael Balzer: > Btw: I've always registered my config "x.rt" as readable, but "config list x.rt" will no longer show any values. I'm pretty sure it did before. Found & fixed, the access rights wouldn't be restored on loading the config.
Regards, Michael
-- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
_______________________________________________ OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev -- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
_______________________________________________ OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
-- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
I think locations does it in the object initialiser, because there is no Init style initialiser for locations (as there is only one location object). If we make these dynamic, we would need an ability to de-register a config parameter as well (presumably in the vehicle destructor). Steve recently made some changes/fixes to config to support ‘orphan’ config parameters (in /store, but not registered), and I think that might impact things here as well. I think it is just simpler to make these parameter registrations permanent, and do them at boot time. If we really want to make them dynamic, then we need to deal with the destructor Unregister case, as well as harden the protection around ‘#access’ style instances. Presumably Command registration has similar issues? How are RT specific commands handled? Regards, Mark.
On 13 Dec 2017, at 6:58 AM, Michael Balzer <dexter@expeedo.de> wrote:
It's actually done both ways in other modules up to now, i.e. the simcom module does it in the Init class, the locations does it in the constructor.
A disadvantage of doing it in the Init classes is, especially if we add instance defaults to the registration, all configs of all modules compiled in will always be listed, regardless of the user actually needing or using them.
Regards, Michael
Am 12.12.2017 um 23:48 schrieb Michael Balzer:
Another option would be to do the RegisterParam() call in the framework registration class, .i.e. OvmsVehicleRenaultTwizyInit. This would assure access rights are set on boot if the module is included, regardless of it being actually started.
That smells a bit like breaking the factory class pattern, but seems to be an alternative.
Regards, Michael
Am 12.12.2017 um 23:35 schrieb Michael Balzer:
It's necessary if we want to be able to see configs of not yet loaded modules, i.e. vehicles. Without the persistance the configs are not readable before the module is actually loaded by the user -- so a module configuration can also not be checked without loading the module.
That was my reason to change this: without persistance, I cannot see my Twizy configuration after boot before issueing "vehicle module RT". If a part of my config needs to be changed before starting the module, I cannot see what I've set before.
You're right in the special meaning of instance names prefixed with "#" -- these should not be allowed for this serialization form. I chose "#" as that's a typical comment line character, and I thought names like this will normally not be used. I'll add a check to prevent using instance names colliding with that pattern, if you agree on the basic solution.
Regards, Michael
Am 12.12.2017 um 23:19 schrieb Mark Webb-Johnson:
It is the second one I am not sure is necessary.
If access is controlled by the Register, why persist at all? What would happen if the permissions in Register were changed? What would happen if an instance "#access" was set?
Regards, Mark
On 13 Dec 2017, at 5:54 AM, Michael Balzer <dexter@expeedo.de> wrote:
I've pushed two commits regarding this, the first only fixed the primary problem of not setting the access rights again on RegisterParam() after loading the config cache.
The second one adds storage persistance, as I think it's OK to persist this info. The RegisterParam() call will always set the current access rights as programmed, this is only meant to restore the latest access rights of parameters registered by loadable modules like vehicles.
So I think this should be OK?
Regards, Michael
Am 12.12.2017 um 22:45 schrieb Mark Webb-Johnson: Michael,
I don't think this is correct/necessary.
The access rights are provided in the Register command, when the parameter is registered. This is done each and every time. We don't need to persist this to /store file system.
There was an issue with load order - where parameters could be loaded before the Register function call. Perhaps that is what is causing the bug you are seeing? The code was fixed to work around this, but perhaps it is not setting the access rights correctly in that case?
Regards, Mark
> On 13 Dec 2017, at 3:32 AM, Michael Balzer <dexter@expeedo.de> wrote: > > >> Am 12.12.2017 um 20:12 schrieb Michael Balzer: >> Btw: I've always registered my config "x.rt" as readable, but "config list x.rt" will no longer show any values. I'm pretty sure it did before. > Found & fixed, the access rights wouldn't be restored on loading the config. > > Regards, > Michael > > -- > Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal > Fon 02333 / 833 5735 * Handy 0176 / 206 989 26 > > _______________________________________________ > OvmsDev mailing list > OvmsDev@lists.teslaclub.hk > http://lists.teslaclub.hk/mailman/listinfo/ovmsdev _______________________________________________ OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev -- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
_______________________________________________ OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
-- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
_______________________________________________ OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
Found a new issue with this today. I think it is caused by the SetAccess() in OvmsConfig::RegisterParam(). That is called during config loading from disk, and parameters are registered there (with default writable, but not readable). The SetAccess() was then modifying the existing parameters to not readable. I added a check to OvmsConfig::mount() to only register parameters found on disk that were not already cached. That should work around it. I’ve also noticed that the entire config system uses std::string (in particular during things like RegisterParam). Given what we now know about temporary std::string objects on the stack when character constants are passed to functions expecting std::string, that is not good. For example, I tried to add a RegisterInstance, and the 8 times I used this in SIMCOM modem initialisation would require 8x3x32 = 768 bytes of temporary stack space. Can we comment-out/remove the #access parameter stuff for the moment, then I will try to change OvmsConfig to use char* for these constants? Regards, Mark.
On 13 Dec 2017, at 8:09 AM, Mark Webb-Johnson <mark@webb-johnson.net> wrote:
I think locations does it in the object initialiser, because there is no Init style initialiser for locations (as there is only one location object).
If we make these dynamic, we would need an ability to de-register a config parameter as well (presumably in the vehicle destructor). Steve recently made some changes/fixes to config to support ‘orphan’ config parameters (in /store, but not registered), and I think that might impact things here as well.
I think it is just simpler to make these parameter registrations permanent, and do them at boot time.
If we really want to make them dynamic, then we need to deal with the destructor Unregister case, as well as harden the protection around ‘#access’ style instances. Presumably Command registration has similar issues? How are RT specific commands handled?
Regards, Mark.
On 13 Dec 2017, at 6:58 AM, Michael Balzer <dexter@expeedo.de> wrote:
It's actually done both ways in other modules up to now, i.e. the simcom module does it in the Init class, the locations does it in the constructor.
A disadvantage of doing it in the Init classes is, especially if we add instance defaults to the registration, all configs of all modules compiled in will always be listed, regardless of the user actually needing or using them.
Regards, Michael
Am 12.12.2017 um 23:48 schrieb Michael Balzer:
Another option would be to do the RegisterParam() call in the framework registration class, .i.e. OvmsVehicleRenaultTwizyInit. This would assure access rights are set on boot if the module is included, regardless of it being actually started.
That smells a bit like breaking the factory class pattern, but seems to be an alternative.
Regards, Michael
Am 12.12.2017 um 23:35 schrieb Michael Balzer:
It's necessary if we want to be able to see configs of not yet loaded modules, i.e. vehicles. Without the persistance the configs are not readable before the module is actually loaded by the user -- so a module configuration can also not be checked without loading the module.
That was my reason to change this: without persistance, I cannot see my Twizy configuration after boot before issueing "vehicle module RT". If a part of my config needs to be changed before starting the module, I cannot see what I've set before.
You're right in the special meaning of instance names prefixed with "#" -- these should not be allowed for this serialization form. I chose "#" as that's a typical comment line character, and I thought names like this will normally not be used. I'll add a check to prevent using instance names colliding with that pattern, if you agree on the basic solution.
Regards, Michael
Am 12.12.2017 um 23:19 schrieb Mark Webb-Johnson:
It is the second one I am not sure is necessary.
If access is controlled by the Register, why persist at all? What would happen if the permissions in Register were changed? What would happen if an instance "#access" was set?
Regards, Mark
On 13 Dec 2017, at 5:54 AM, Michael Balzer <dexter@expeedo.de> wrote:
I've pushed two commits regarding this, the first only fixed the primary problem of not setting the access rights again on RegisterParam() after loading the config cache.
The second one adds storage persistance, as I think it's OK to persist this info. The RegisterParam() call will always set the current access rights as programmed, this is only meant to restore the latest access rights of parameters registered by loadable modules like vehicles.
So I think this should be OK?
Regards, Michael
> Am 12.12.2017 um 22:45 schrieb Mark Webb-Johnson: > Michael, > > I don't think this is correct/necessary. > > The access rights are provided in the Register command, when the parameter is registered. This is done each and every time. We don't need to persist this to /store file system. > > There was an issue with load order - where parameters could be loaded before the Register function call. Perhaps that is what is causing the bug you are seeing? The code was fixed to work around this, but perhaps it is not setting the access rights correctly in that case? > > Regards, Mark > >> On 13 Dec 2017, at 3:32 AM, Michael Balzer <dexter@expeedo.de> wrote: >> >> >>> Am 12.12.2017 um 20:12 schrieb Michael Balzer: >>> Btw: I've always registered my config "x.rt" as readable, but "config list x.rt" will no longer show any values. I'm pretty sure it did before. >> Found & fixed, the access rights wouldn't be restored on loading the config. >> >> Regards, >> Michael >> >> -- >> Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal >> Fon 02333 / 833 5735 * Handy 0176 / 206 989 26 >> >> _______________________________________________ >> OvmsDev mailing list >> OvmsDev@lists.teslaclub.hk >> http://lists.teslaclub.hk/mailman/listinfo/ovmsdev > _______________________________________________ > OvmsDev mailing list > OvmsDev@lists.teslaclub.hk > http://lists.teslaclub.hk/mailman/listinfo/ovmsdev -- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
_______________________________________________ OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
-- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
_______________________________________________ OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
I've added an encapsulation by using an inline #define in ovms_config.cpp, by default commented out. The "#access" line if already present will be ignored by the loader. Regards, Michael Am 13.12.2017 um 07:41 schrieb Mark Webb-Johnson:
Found a new issue with this today. I think it is caused by the SetAccess() in OvmsConfig::RegisterParam(). That is called during config loading from disk, and parameters are registered there (with default writable, but not readable). The SetAccess() was then modifying the existing parameters to not readable.
I added a check to OvmsConfig::mount() to only register parameters found on disk that were not already cached. That should work around it.
I’ve also noticed that the entire config system uses std::string (in particular during things like RegisterParam). Given what we now know about temporary std::string objects on the stack when character constants are passed to functions expecting std::string, that is not good. For example, I tried to add a RegisterInstance, and the 8 times I used this in SIMCOM modem initialisation would require 8x3x32 = 768 bytes of temporary stack space.
Can we comment-out/remove the #access parameter stuff for the moment, then I will try to change OvmsConfig to use char* for these constants?
Regards, Mark.
On 13 Dec 2017, at 8:09 AM, Mark Webb-Johnson <mark@webb-johnson.net> wrote:
I think locations does it in the object initialiser, because there is no Init style initialiser for locations (as there is only one location object).
If we make these dynamic, we would need an ability to de-register a config parameter as well (presumably in the vehicle destructor). Steve recently made some changes/fixes to config to support ‘orphan’ config parameters (in /store, but not registered), and I think that might impact things here as well.
I think it is just simpler to make these parameter registrations permanent, and do them at boot time.
If we really want to make them dynamic, then we need to deal with the destructor Unregister case, as well as harden the protection around ‘#access’ style instances. Presumably Command registration has similar issues? How are RT specific commands handled?
Regards, Mark.
On 13 Dec 2017, at 6:58 AM, Michael Balzer <dexter@expeedo.de> wrote:
It's actually done both ways in other modules up to now, i.e. the simcom module does it in the Init class, the locations does it in the constructor.
A disadvantage of doing it in the Init classes is, especially if we add instance defaults to the registration, all configs of all modules compiled in will always be listed, regardless of the user actually needing or using them.
Regards, Michael
Am 12.12.2017 um 23:48 schrieb Michael Balzer:
Another option would be to do the RegisterParam() call in the framework registration class, .i.e. OvmsVehicleRenaultTwizyInit. This would assure access rights are set on boot if the module is included, regardless of it being actually started.
That smells a bit like breaking the factory class pattern, but seems to be an alternative.
Regards, Michael
Am 12.12.2017 um 23:35 schrieb Michael Balzer:
It's necessary if we want to be able to see configs of not yet loaded modules, i.e. vehicles. Without the persistance the configs are not readable before the module is actually loaded by the user -- so a module configuration can also not be checked without loading the module.
That was my reason to change this: without persistance, I cannot see my Twizy configuration after boot before issueing "vehicle module RT". If a part of my config needs to be changed before starting the module, I cannot see what I've set before.
You're right in the special meaning of instance names prefixed with "#" -- these should not be allowed for this serialization form. I chose "#" as that's a typical comment line character, and I thought names like this will normally not be used. I'll add a check to prevent using instance names colliding with that pattern, if you agree on the basic solution.
Regards, Michael
Am 12.12.2017 um 23:19 schrieb Mark Webb-Johnson:
It is the second one I am not sure is necessary.
If access is controlled by the Register, why persist at all? What would happen if the permissions in Register were changed? What would happen if an instance "#access" was set?
Regards, Mark
> On 13 Dec 2017, at 5:54 AM, Michael Balzer <dexter@expeedo.de> wrote: > > I've pushed two commits regarding this, the first only fixed the primary problem of not setting the access rights again on RegisterParam() after loading the > config cache. > > The second one adds storage persistance, as I think it's OK to persist this info. The RegisterParam() call will always set the current access rights as > programmed, this is only meant to restore the latest access rights of parameters registered by loadable modules like vehicles. > > So I think this should be OK? > > Regards, > Michael > > >> Am 12.12.2017 um 22:45 schrieb Mark Webb-Johnson: >> Michael, >> >> I don't think this is correct/necessary. >> >> The access rights are provided in the Register command, when the parameter is registered. This is done each and every time. We don't need to persist this to /store file system. >> >> There was an issue with load order - where parameters could be loaded before the Register function call. Perhaps that is what is causing the bug you are seeing? The code was fixed to work around this, but perhaps it is not setting the access rights correctly in that case? >> >> Regards, Mark >> >>> On 13 Dec 2017, at 3:32 AM, Michael Balzer <dexter@expeedo.de> wrote: >>> >>> >>>> Am 12.12.2017 um 20:12 schrieb Michael Balzer: >>>> Btw: I've always registered my config "x.rt" as readable, but "config list x.rt" will no longer show any values. I'm pretty sure it did before. >>> Found & fixed, the access rights wouldn't be restored on loading the config. >>> >>> Regards, >>> Michael >>> >>> -- >>> Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal >>> Fon 02333 / 833 5735 * Handy 0176 / 206 989 26 >>> >>> _______________________________________________ >>> OvmsDev mailing list >>> OvmsDev@lists.teslaclub.hk >>> http://lists.teslaclub.hk/mailman/listinfo/ovmsdev >> _______________________________________________ >> OvmsDev mailing list >> OvmsDev@lists.teslaclub.hk >> http://lists.teslaclub.hk/mailman/listinfo/ovmsdev > -- > Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal > Fon 02333 / 833 5735 * Handy 0176 / 206 989 26 > > > _______________________________________________ > OvmsDev mailing list > OvmsDev@lists.teslaclub.hk > http://lists.teslaclub.hk/mailman/listinfo/ovmsdev _______________________________________________ OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev -- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
_______________________________________________ OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
-- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
I think: * framework modules should register their configs, commands & metrics on init * pluggable modules should register their configs, commands & metrics on load * once a user has actually set configs for pluggable modules, he/she should be able to see and modify them also when the module isn't loaded That's what the current implementation now provides (except I've missed adding the config title to the storage meta data section as well). This keeps both RAM usage as low as possible and the name spaces as clean as possible. On the Twizy module I've implemented this approach, Geir has done it likewise. Config deregistration is not really necessary unless a module wants to hide its config. Or at least DeregisterParam() / "config rm" currently works that way, it doesn't delete the storage file. Also, instance names actually do not need to be restricted. My storage meta data syntax uses '=' as the value separator, instances use tab, so we can easily tell them apart. Regards, Michael Am 13.12.2017 um 01:09 schrieb Mark Webb-Johnson:
I think locations does it in the object initialiser, because there is no Init style initialiser for locations (as there is only one location object).
If we make these dynamic, we would need an ability to de-register a config parameter as well (presumably in the vehicle destructor). Steve recently made some changes/fixes to config to support ‘orphan’ config parameters (in /store, but not registered), and I think that might impact things here as well.
I think it is just simpler to make these parameter registrations permanent, and do them at boot time.
If we really want to make them dynamic, then we need to deal with the destructor Unregister case, as well as harden the protection around ‘#access’ style instances. Presumably Command registration has similar issues? How are RT specific commands handled?
Regards, Mark.
On 13 Dec 2017, at 6:58 AM, Michael Balzer <dexter@expeedo.de> wrote:
It's actually done both ways in other modules up to now, i.e. the simcom module does it in the Init class, the locations does it in the constructor.
A disadvantage of doing it in the Init classes is, especially if we add instance defaults to the registration, all configs of all modules compiled in will always be listed, regardless of the user actually needing or using them.
Regards, Michael
Am 12.12.2017 um 23:48 schrieb Michael Balzer:
Another option would be to do the RegisterParam() call in the framework registration class, .i.e. OvmsVehicleRenaultTwizyInit. This would assure access rights are set on boot if the module is included, regardless of it being actually started.
That smells a bit like breaking the factory class pattern, but seems to be an alternative.
Regards, Michael
Am 12.12.2017 um 23:35 schrieb Michael Balzer:
It's necessary if we want to be able to see configs of not yet loaded modules, i.e. vehicles. Without the persistance the configs are not readable before the module is actually loaded by the user -- so a module configuration can also not be checked without loading the module.
That was my reason to change this: without persistance, I cannot see my Twizy configuration after boot before issueing "vehicle module RT". If a part of my config needs to be changed before starting the module, I cannot see what I've set before.
You're right in the special meaning of instance names prefixed with "#" -- these should not be allowed for this serialization form. I chose "#" as that's a typical comment line character, and I thought names like this will normally not be used. I'll add a check to prevent using instance names colliding with that pattern, if you agree on the basic solution.
Regards, Michael
Am 12.12.2017 um 23:19 schrieb Mark Webb-Johnson:
It is the second one I am not sure is necessary.
If access is controlled by the Register, why persist at all? What would happen if the permissions in Register were changed? What would happen if an instance "#access" was set?
Regards, Mark
On 13 Dec 2017, at 5:54 AM, Michael Balzer <dexter@expeedo.de> wrote:
I've pushed two commits regarding this, the first only fixed the primary problem of not setting the access rights again on RegisterParam() after loading the config cache.
The second one adds storage persistance, as I think it's OK to persist this info. The RegisterParam() call will always set the current access rights as programmed, this is only meant to restore the latest access rights of parameters registered by loadable modules like vehicles.
So I think this should be OK?
Regards, Michael
> Am 12.12.2017 um 22:45 schrieb Mark Webb-Johnson: > Michael, > > I don't think this is correct/necessary. > > The access rights are provided in the Register command, when the parameter is registered. This is done each and every time. We don't need to persist this to /store file system. > > There was an issue with load order - where parameters could be loaded before the Register function call. Perhaps that is what is causing the bug you are seeing? The code was fixed to work around this, but perhaps it is not setting the access rights correctly in that case? > > Regards, Mark > >> On 13 Dec 2017, at 3:32 AM, Michael Balzer <dexter@expeedo.de> wrote: >> >> >>> Am 12.12.2017 um 20:12 schrieb Michael Balzer: >>> Btw: I've always registered my config "x.rt" as readable, but "config list x.rt" will no longer show any values. I'm pretty sure it did before. >> Found & fixed, the access rights wouldn't be restored on loading the config. >> >> Regards, >> Michael >> >> -- >> Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal >> Fon 02333 / 833 5735 * Handy 0176 / 206 989 26 >> >> _______________________________________________ >> OvmsDev mailing list >> OvmsDev@lists.teslaclub.hk >> http://lists.teslaclub.hk/mailman/listinfo/ovmsdev > _______________________________________________ > OvmsDev mailing list > OvmsDev@lists.teslaclub.hk > http://lists.teslaclub.hk/mailman/listinfo/ovmsdev -- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
_______________________________________________ OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev -- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
_______________________________________________ OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
OvmsDev mailing list OvmsDev@lists.teslaclub.hk http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
-- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
participants (3)
-
Geir Øyvind Vælidalo -
Mark Webb-Johnson -
Michael Balzer