Hi Wolfgang, On Sat, Nov 28, 2009 at 09:17:57PM +0100, Wolfgang Grandegger wrote: > Hi Fu, > > Luotao Fu wrote: > > Hi Wolfgang, > > > > On Fri, Nov 27, 2009 at 11:14:57AM +0100, Wolfgang Grandegger wrote: > > ... > >> I don't want to invite people playing with the sample-point. To be > >> clear, sample_point==0 will not be ignored. It means using CIA > >> recommended sample points for the specified bit-rate. > >> > > > > The function is now splitte up in a simple set_bitrate and a expert > > set_bitrate_samplepoint as you suggested. I Also changed name prefix > > to can_ in API and renamed the library to libsocketcan. Hence the lib > > is now available at: > > git://git.pengutronix.de/git/tools/libsocketcan.git > > The old URL still exists as a link for convenience though. > > > > I pinned the lib version to 0.0.4 and the updated canutils to 4.0.2. > > There're tarballs for ready to use source on our ftp server. > > I finally had a closer look. I have attached two patches showing my > enhancements and fixes.
Thanks for the patches, comment see below. > Futhermore, I would change the name "mode" to "ctrlmode". "mode" is > reserved for CAN operation modes like start, stop, or sleep. agreed. > It would also be nice to allow: > > $ canconfig can0 bitrate 125000 restart-ms 100 start > This'd be indeed nice to have. Though we might have to change the parameter parsing quite a littel. Let's see... > Add can_get_bittiming_const() > > Signed-off-by: Wolfgang Grandegger <[email protected]> Acked-by: Luotao Fu <[email protected]> applied, thx. <snip> > Add can_get_bittiming_const() and further fixes and improvements > > - s/can_modes/can_states/ > - Use proper names for CAN error states > - Use "-" as name seperator for input and output consequently > - Add some more useful output to cmd_show_interface() > > Signed-off-by: Wolfgang Grandegger <[email protected]> > minor nitpick see below. otherwise Acked-by: Luotao Fu <[email protected]> > diff --git a/src/canconfig.c b/src/canconfig.c > index d8b9167..0139a1c 100644 > --- a/src/canconfig.c > +++ b/src/canconfig.c > @@ -37,11 +37,11 @@ > #define MIN(a, b) ((a) < (b) ? (a) : (b)) > #endif > > -const char *can_modes[CAN_STATE_MAX] = { > - "ACTIVE", > - "WARNING", > - "PASSIVE", > - "BUS OFF", > +const char *can_states[CAN_STATE_MAX] = { > + "ERROR-ACTIVE", > + "ERROR-WARNING", > + "ERROR-PASSIVE", > + "BUS-OFF", > "STOPPED", > "SLEEPING" > }; > @@ -59,23 +59,24 @@ const char *can_modes[CAN_STATE_MAX] = { > static void help(void) > { > fprintf(stderr, "usage:\n\t" > - "canconfig <dev> bitrate { BR } [sample_point { SP }]\n\t\t" > + "canconfig <dev> bitrate { BR } [sample-point { SP }]\n\t\t" > "BR := <bitrate in Hz>\n\t\t" > - "SP := <sample point {0...0.999}> (optional)\n\t" > + "SP := <sample-point {0...0.999}> (optional)\n\t" > "canconfig <dev> bittiming [ VALs ]\n\t\t" > "VALs := <tq | prop-seg | phase-seg1 | phase-seg2 | sjw>\n\t\t" > "tq <time quantum in ns>\n\t\t" > - "prop_seg <no. in tq>\n\t\t" > - "phase_seg1 <no. in tq>\n\t\t" > - "phase_seg2 <no. in tq\n\t\t" > + "prop-seg <no. in tq>\n\t\t" > + "phase-seg1 <no. in tq>\n\t\t" > + "phase-seg2 <no. in tq\n\t\t" > "sjw <no. in tq> (optional)\n\t" > - "canconfig <dev> restart-ms { RESTART_MS }\n\t\t" > - "RESTART_MS := <autorestart interval in ms>\n\t" > + "canconfig <dev> restart-ms { RESTART-MS }\n\t\t" > + "RESTART-MS := <autorestart interval in ms>\n\t" > "canconfig <dev> mode { MODE }\n\t\t" > "MODE := <[loopback | listen-only | triple-sampling] > [on|off]>\n\t" > "canconfig <dev> {ACTION}\n\t\t" > - "ACTION := <[start|stop|restart]>" > - "canconfig <dev> clockfreq\n" > + "ACTION := <[start|stop|restart]>\n\t" > + "canconfig <dev> clockfreq\n\t" > + "canconfig <dev> bittiming-constants\n" > ); > > exit(EXIT_FAILURE); > @@ -90,7 +91,7 @@ static void do_show_bitrate(int argc, char* argv[]) > exit(EXIT_FAILURE); > } else > fprintf(stdout, > - "%s bitrate: %u, sample point: %0.3f\n", > + "%s bitrate: %u, sample-point: %0.3f\n", > argv[1], bt.bitrate, > (float)((float)bt.sample_point / 1000)); > } > @@ -202,6 +203,24 @@ static void do_show_bittiming(int argc, char *argv[]) > bt.sjw, bt.brp); > } > > +static void do_show_bittiming_const(int argc, char *argv[]) > +{ > + const char *name = argv[1]; > + struct can_bittiming_const btc; > + > + if (can_get_bittiming_const(name, &btc) < 0) { > + fprintf(stderr, "%s: failed to get bittiming_const\n", argv[1]); > + exit(EXIT_FAILURE); > + } else > + fprintf(stdout, "%s bittiming-constants: name %s,\n\t" > + "tseg1_min: %u, tseg1_max: %u, " > + "tseg2_min: %u, tseg2_max: %u,\n\t" > + "sjw_max %u, brp_min: %u, brp_max: %u, brp_inc: %u,\n", > + name, btc.name, btc.tseg1_min, btc.tseg1_max, > + btc.tseg2_min, btc.tseg2_max, btc.sjw_max, > + btc.brp_min, btc.brp_max, btc.brp_inc); > +} > + to be consequent, we might also want "-"s instead of "_"s in the print too. If you agree I'll change this here directly to avoid a resend. > static void cmd_bittiming(int argc, char *argv[]) > { > if (argc >= 4) { > @@ -223,7 +242,7 @@ static void do_show_state(int argc, char *argv[]) > } > > if (state >= 0 && state < CAN_STATE_MAX) > - fprintf(stdout, "%s state: %s\n", argv[1], can_modes[state]); > + fprintf(stdout, "%s state: %s\n", argv[1], can_states[state]); > else > fprintf(stderr, "%s: unknown state\n", argv[1]); > } > @@ -257,6 +276,13 @@ static void cmd_clockfreq(int argc, char *argv[]) > exit(EXIT_SUCCESS); > } > > +static void cmd_bittiming_const(int argc, char *argv[]) > +{ > + do_show_bittiming_const(argc, argv); > + > + exit(EXIT_SUCCESS); > +} > + > static void do_restart(int argc, char *argv[]) > { > if (can_do_restart(argv[1]) < 0) { > @@ -395,7 +421,7 @@ static void do_show_restart_ms(int argc, char* argv[]) > exit(EXIT_FAILURE); > } else > fprintf(stdout, > - "%s restart_ms: %u\n", argv[1], restart_ms); > + "%s restart-ms: %u\n", argv[1], restart_ms); > } > > static void do_set_restart_ms(int argc, char* argv[]) > @@ -430,10 +456,12 @@ static void cmd_baudrate(int argc, char *argv[]) > static void cmd_show_interface(int argc, char *argv[]) > { > do_show_bitrate(argc, argv); > + do_show_bittiming(argc, argv); > do_show_state(argc, argv); > do_show_restart_ms(argc, argv); > - do_show_clockfreq(argc, argv); > do_show_ctrlmode(argc, argv); > + do_show_clockfreq(argc, argv); > + do_show_bittiming_const(argc, argv); > I kept the bittiming information out of show_interface to keep the common output simple as possible. But you are so far right that these are also essential informations. I might later add a verbose mode for all these informations. In the common mode only the most important infos like bitrate/state will be displayed than. > exit(EXIT_SUCCESS); > } > @@ -467,6 +495,8 @@ int main(int argc, char *argv[]) > cmd_state(argc, argv); > if (!strcmp(argv[2], "clockfreq")) > cmd_clockfreq(argc, argv); > + if (!strcmp(argv[2], "bittiming-constants")) > + cmd_bittiming_const(argc, argv); > > help(); > Thanks cheers Luotao Fu -- Pengutronix e.K. | Dipl.-Ing. Luotao Fu | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
signature.asc
Description: Digital signature
_______________________________________________ Socketcan-users mailing list [email protected] https://lists.berlios.de/mailman/listinfo/socketcan-users
