tinyos / nesc

Master nesc repository
GNU General Public License v2.0
100 stars 53 forks source link

New nesC Flags - OR - How Implicit Connections are Actually an Anti-feature #8

Closed bradjc closed 11 years ago

bradjc commented 11 years ago

I propose two new flags be added to nesC.

1. Disable Implicit Connections

The first would disable implicit connections (section 9.3 in the manual). While the motivation for this feature is clear, using it leads to code that is very hard to parse by humans. Because interfaces can be renamed between files, tracking how components are wired together when implicit connections are used can be intractable. For example:

Component1C.nc:
configuration {
  provides interface Gpio as UsefulPin;
  provides interface Gpio2 as OkPin;
  provides interface Gpio3 as BadPin;
  provides interface Gpio4 as InterruptPin;
}
implementation { ... }

Component2C.nc:
configuration {
  provides interface Gpio as GoodPin;
}
implementation {
  components Component1C;
  Goodpin = Component1C;
}

Now say one wants to use GoodPin from Component2C. While this example is pretty contrived, determining what code is actually running when one uses GoodPin from Component2C is difficult. After seeing that GoodPin is provided by Component1C, one goes to Component1C.nc to find what it connects to, and of course sees nothing matching "GoodPin". Then one must go back to Component2C.nc and determine the actual interface of GoodPin and match that interface in Component1C.nc. From now on, one must just remember that those pins are connected. This, however, becomes impossible when one needs to traverse up and down the connections. Much of this pain can be relieved by being able to easily match names between files, and not having to remember all of the implicit connections.

Now because TinyOS uses implicit connections extensively, support cannot just be removed. But by adding a flag to cause them to generate compile errors (that include what the connection actually is), one could systematically remove them, saving future developers much headache.

2. Force Filename Conventions

The second flag would require that configuration components be in files with filenames that end in C.nc, module components be in filenames that end in P.nc, and interfaces be in files with filenames that do not end with either P.nc or C.nc. Again, this is a code readability issue. It is much easier to follow code that follows these rules, and compiling with a flag that generates errors when this convention is broken would make sure that code conforms.

cire831 commented 11 years ago

Both of these are good suggestions.

Would you be willing to tackle the coding?

On Fri, Apr 26, 2013 at 2:12 PM, Brad Campbell notifications@github.comwrote:

I propose two new flags be added to nesC.

  1. Disable Implicit Connections

The first would disable implicit connections (section 9.3 in the manual). While the motivation for this feature is clear, using it leads to code that is very hard to parse by humans. Because interfaces can be renamed between files, tracking how components are wired together when implicit connections are used can be intractable. For example:

Component1C.nc: configuration { provides interface Gpio as UsefulPin; provides interface Gpio2 as OkPin; provides interface Gpio3 as BadPin; provides interface Gpio4 as InterruptPin; } implementation { ... }

Component2C.nc: configuration { provides interface Gpio as GoodPin; } implementation { components Component1C; Goodpin = Component1C; }

Now say one wants to use GoodPin from Component2C. While this example is pretty contrived, determining what code is actually running when one uses GoodPin from Component2C is difficult. After seeing that GoodPin is provided by Component1C, one goes to Component1C.nc to find what it connects to, and of course sees nothing matching "GoodPin". Then one must go back to Component2C.nc and determine the actual interface of GoodPinand match that interface in Component1C.nc. From now on, one must just remember that those pins are connected. This, however, becomes impossible when one needs to traverse up and down the connections. Much of this pain can be relieved by being able to easily match names between files, and not having to remember all of the implicit connections.

Now because TinyOS uses implicit connections extensively, support cannot just be removed. But by adding a flag to cause them to generate compile errors (that include what the connection actually is), one could systematically remove them, saving future developers much headache.

  1. Force Filename Conventions

The second flag would require that configuration components be in files with filenames that end in C.nc, module components be in filenames that end in P.nc, and interfaces be in files with filenames that do not end with either P.nc or C.nc. Again, this is a code readability issue. It is much easier to follow code that follows these rules, and compiling with a flag that generates errors when this convention is broken would make sure that code conforms.

— Reply to this email directly or view it on GitHubhttps://github.com/tinyos/nesc/issues/8 .

Eric B. Decker Senior (over 50 :-) Researcher

bradjc commented 11 years ago

I'm not sure I'll be able to get to it right away, but I'm willing to take a crack at it.

cire831 commented 11 years ago

cool. it isn't going to disappear so whenever you can.

On Fri, Apr 26, 2013 at 10:54 PM, Brad Campbell notifications@github.comwrote:

I'm not sure I'll be able to get to it right away, but I'm willing to take a crack at it.

— Reply to this email directly or view it on GitHubhttps://github.com/tinyos/nesc/issues/8#issuecomment-17111096 .

Eric B. Decker Senior (over 50 :-) Researcher

dgay42 commented 11 years ago

On 2013-04-26 22:18, Eric Decker wrote:

Both of these are good suggestions.

Would you be willing to tackle the coding?

On Fri, Apr 26, 2013 at 2:12 PM, Brad Campbell notifications@github.comwrote:

I propose two new flags be added to nesC.

  1. Disable Implicit Connections

The first would disable implicit connections (section 9.3 in the manual). While the motivation for this feature is clear, using it leads to code that is very hard to parse by humans. Because interfaces can be renamed between files, tracking how components are wired together when implicit connections are used can be intractable. For example:

Component1C.nc: configuration { provides interface Gpio as UsefulPin; provides interface Gpio2 as OkPin; provides interface Gpio3 as BadPin; provides interface Gpio4 as InterruptPin; } implementation { ... }

Component2C.nc: configuration { provides interface Gpio as GoodPin; } implementation { components Component1C; Goodpin = Component1C; }

Now say one wants to use GoodPin from Component2C. While this example is pretty contrived, determining what code is actually running when one uses GoodPin from Component2C is difficult. After seeing that GoodPin is

provided by Component1C, one goes to Component1C.nc to find what it

connects to, and of course sees nothing matching "GoodPin". Then one must go back to Component2C.nc and determine the actual interface of GoodPinand match that interface in Component1C.nc. From now on, one must just remember that those pins are connected. This, however, becomes impossible when one needs to traverse up and down the connections. Much of this pain can be relieved by being able to easily match names between files, and not having to remember all of the implicit connections.

I'm not sure I disagree, but I will point out the error here was to match by interface name and not type (the first step should have been "look for an interface of type Gpio in Component1C).

Now because TinyOS uses implicit connections extensively, support cannot just be removed. But by adding a flag to cause them to generate compile errors (that include what the connection actually is), one could systematically remove them, saving future developers much headache.

  1. Force Filename Conventions

The second flag would require that configuration components be in files with filenames that end in C.nc, module components be in filenames that end in P.nc, and interfaces be in files with filenames that do not end with either P.nc or C.nc. Again, this is a code readability issue. It is much easier to follow code that follows these rules, and compiling with a flag that generates errors when this convention is broken would make sure that code conforms.

This exact rule is wrong, as P stands for "private", not module (in fact we explicitly moved away from a C and M convention some years ago).

Suggestion: components must end in C or P, independent of whether they are modules or configurations.

David Gay

— Reply to this email directly or view it on GitHubhttps://github.com/tinyos/nesc/issues/8 .

-- Eric B. Decker Senior (over 50 :-) Researcher

Reply to this email directly or view it on GitHub [1].

Links:

[1] https://github.com/tinyos/nesc/issues/8#issuecomment-17110758

bradjc commented 11 years ago

I'm not sure I disagree, but I will point out the error here was to match by interface name and not type (the first step should have been "look for an interface of type Gpio in Component1C).

That's fair, but often the actual interface is specified in yet a different file, causing a different memorize-and-follow problem.

This exact rule is wrong, as P stands for "private", not module (in fact we explicitly moved away from a C and M convention some years ago). Suggestion: components must end in C or P, independent of whether they are modules or configurations.

Ah, ok. It seems this is more of a TinyOS issue then. I think it would be useful to be able to distinguish both configuration vs. module and public vs. private from the filename. But given this organization I don't think there is much a compiler can enforce that would aid code parsability and readability.

bradjc commented 11 years ago

After some more thought, a standard naming scheme that merges the C/M and C/P styles would be useful, and the compiler could help enforce. By adding L.nc (L for 'library') files, public modules could have their own namespace. That way all wiring is done with C.nc files, all private code is P.nc and all more general code is L.nc files. nesC can easily check if only configurations are in C.nc, and could issue warnings if any P.nc files are used that are not in the current directory or child directories. I think this would help make nesC code more intuitive, and help people read and understand modules they didn't write.

cire831 commented 11 years ago

What do we do with all the older files?

The transition would be a bitch.

Flag day?

On Monday, April 29, 2013, Brad Campbell wrote:

After some more thought, a standard naming scheme that merges the C/M and C/P styles would be useful, and the compiler could help enforce. By adding L.nc (L for 'library') files, public modules could have their own namespace. That way all wiring is done with C.nc files, all private code is P.nc and all more general code is L.nc files. nesC can easily check if only configurations are in C.nc, and could issue warnings if any P.ncfiles are used that are not in the current directory or child directories. I think this would help make nesC code more intuitive, and help people read and understand modules they didn't write.

— Reply to this email directly or view it on GitHubhttps://github.com/tinyos/nesc/issues/8#issuecomment-17189027 .

Eric B. Decker Senior (over 50 :-) Researcher

dgay42 commented 11 years ago

On 2013-04-29 12:55, Eric Decker wrote:

What do we do with all the older files?

The transition would be a bitch.

Flag day?

On Monday, April 29, 2013, Brad Campbell wrote:

After some more thought, a standard naming scheme that merges the C/M and C/P styles would be useful, and the compiler could help enforce. By adding L.nc (L for library) files, public modules could have their own namespace. That way all wiring is done with C.nc files, all private code is P.nc and all more general code is L.nc files. nesC can easily check if only configurations are in C.nc, and could issue warnings if any P.ncfiles are used that are not in the current directory or child directories. I think this would help make nesC code more intuitive, and help people read and understand modules they didnt write.

The C/M convention was only used for TinyOS 1 AFAIK. So it should be ignored.

The user of a component shouldn't care whether it's a configuration or a module, so any naming convention that exposes that distinction is broken (ex: I should be able to have a public module, then decide later to split into two parts that are wired together by a configuration and not have the user's of the public now-configuration have to change anything).

David

— Reply to this email directly or view it on GitHubhttps://github.com/tinyos/nesc/issues/8#issuecomment-17189027 .

-- Eric B. Decker Senior (over 50 :-) Researcher

Reply to this email directly or view it on GitHub [1].

Links:

[1] https://github.com/tinyos/nesc/issues/8#issuecomment-17190072

bradjc commented 11 years ago

The C/M convention was only used for TinyOS 1 AFAIK. So it should be ignored. The user of a component shouldn't care whether it's a configuration or a module, so any naming convention that exposes that distinction is broken (ex: I should be able to have a public module, then decide later to split into two parts that are wired together by a configuration and not have the user's of the public now-configuration have to change anything).

OK that makes sense and I've even used that setup to my advantage. There seems to be a conflict between assisting someone who wants to wire different modules together, and someone who wants to be able to understand a single module.

One solution that I don't think is very popular is to require configuration files for every module, no matter how trivial. Currently, nesC essentially provides an implicit configuration when a module is in a C.nc file. Forcing all code (modules) to be private and requiring the implicit configuration to be explicit, would balance the niceties of code filenames corresponding to their contents with maintaining a public/private divide at the expense of adding a bunch of rather worthless configuration files that simply pass through interfaces.

What might work instead are two changes that allow developers to name their nesC files whatever they want.

  1. Make public and private keywords that describe components, with components defaulting to private. Private components would only be available if they were located in the same directory or in a subdirectory. This syntax makes sense if the compiler is to help enforce it. (This could also have the side benefit of allowing developers to make clear [by specifying as public] which components in a module a different module should wire to.)
  2. Allow configurations and modules to be named the same. Then nesC would use the explicit configuration if available, otherwise it would default to the implicit configuration based on the module of the correct name. Then when one wires components together, one simply refers to the name without any trailing 'C'. Then creating a configuration where there wasn't one previously will just work, as it does now. The only special case would be when a configuration wires to "itself" it would actual wire to the module of the same name.

In this setup, filenames don't matter. They can be whatever the developer wants. The timer virtualization code could be called VirtualizeTimerPubM.nc. File naming would be strictly project style dependent. However, nesC flags could require that only 1 component be in each file and/or that the component name be the beginning of the file name.

vlahan commented 11 years ago

One of the benefits of the C/P convention was that one can easily differentiate between modules/configurations and pure interface definition files. If one hides the C/P endings, we need to also deal with the interface file naming.

bradjc commented 11 years ago

The endings could be as simple or descriptive as the author would like because the filename matching the module name is no longer a requirement. Interface files could easily stay without any prefix, while modules/configurations could be marked as the type of component and its public/private status.

phil-levis commented 11 years ago

Let's back up here -- the problem isn't a language one, it's a language use one. The stated problem is that there are too many levels of indirection, which makes code really hard to read, something I would completely agree with. We can try to change the language to make this harder, or just refactor the code to make it clearer. Part of these levels of indirection come from when TinyOS development was about figuring out the right interfaces and levels of abstraction. This meant we used lots of tiny components. Nowadays, things are much more settled, and so having a smaller number of larger components might make the code easier to follow.

So rather than change the language (which would then require refactoring all of the code), why not just refactor the code to make it simpler?

cire831 commented 11 years ago

On Fri, May 3, 2013 at 2:09 PM, phil-levis notifications@github.com wrote:

Let's back up here -- the problem isn't a language one, it's a language use one. The stated problem is that there are too many levels of indirection, which makes code really hard to read, something I would completely agree with. We can try to change the language to make this harder, or just refactor the code to make it clearer. Part of these levels of indirection come from when TinyOS development was about figuring out the right interfaces and levels of abstraction. This meant we used lots of tiny components. Nowadays, things are much more settled, and so having a smaller number of larger components might make the code easier to follow.

So rather than change the language (which would then require refactoring all of the code), why not just refactor the code to make it simpler?

I agree. nesc and TinyOS are pretty powerful and like C can be used to shoot oneself in the foot. But used in a reasonable fashion can work really well.

— Reply to this email directly or view it on GitHubhttps://github.com/tinyos/nesc/issues/8#issuecomment-17418365 .

Eric B. Decker Senior (over 50 :-) Researcher

bradjc commented 11 years ago

So rather than change the language (which would then require refactoring all of the code), why not just refactor the code to make it simpler?

This is the main reason I originally imagined these changes as flags: they would help developers write "better" code but not necessarily require it. My motivation is to make it easier for new developers to come up to speed when writing new drivers, platforms, etc., and I think this starts with making the code easier to follow. Refactoring obtuse components would definitely help, but so would be able to scan a directory for all of the glue components (aka configurations) to see how everything is connected.

I only make configurations public (and create the implicit ones if necessary) because I think this makes drivers easier to read. I think the best way to balance public/private components, descriptive filenames, readable code, and easily over-ridable components is with language changes, but that doesn't mean its strictly necessary.

dgay42 commented 11 years ago

Implicit connection warning flag in 1.3.5.